Simple TicTacToe game, played by two simple A.I. players I made two player objects (p1 and p2):

public class tttmodimproved{
    static int turn=0;
    public static void main(String[] args){
        System.out.println("Let's play TicTacToe");
        char isWin=' ';
        Board b= new Board();
        Player p1=new Player('X');
        Player p2= new Player('O');
        Player p;
        for(b.tc=0;b.tc<9;b.tc++){
            p=cp(p1,p2);
            b.updateBoard(p,p.getMove(b));
            isWin=b.checkWin(p);
            b.showBoard();
            if(isWin!=' ')
                break;
        }
         if(isWin!=' ')
            System.out.println("congrats! " + isWin +" won the game.");
        else
            System.out.println("tie");

    }
    static Player cp(Player p1,Player p2){

        if(turn==0)
        {
            turn=1;
            return p1;
        }
        else if(turn==1)
        {
            turn=0;
            return p2;
        }
        else
            return p1;
    }
    }
    class Board{
    char[] s={' ',' ',' ',' ',' ',' ',' ',' ',' '};
    int tc=0;

    char checkWin(Player p){
        char m=p.mark;
        if(m==((s[1]&s[4]&s[7])
            |(s[0]&s[3]&s[6])
            |(s[2]&s[5]&s[8])
            |(s[6]&s[7]&s[8])
            |(s[3]&s[4]&s[5])
            |(s[0]&s[1]&s[2])
            |(s[2]&s[4]&s[6])
            |(s[0]&s[4]&s[8]))){
            return m;}
        return (' ');
    }
    void showBoard(){
        System.out.println("");
        System.out.println(" "+s[6]+" | "+s[7]+" | "+s[8]+" ");
        System.out.println("---|---|---");
        System.out.println(" "+s[3]+" | "+s[4]+" | "+s[5]+" ");
        System.out.println("---|---|---");
        System.out.println(" "+s[0]+" | "+s[1]+" | "+s[2]+" ");
        System.out.println("");
    }
    void updateBoard(Player p,int move){
        s[move]=p.mark;
    }

    }
    class Player{
    char mark=' ',markOpo=' ';

    Player(char mark){
        this.mark=mark;
        System.out.println("player created with mark: " +this.mark);
        if(this.mark=='X'){
            markOpo='O';
        }
        else
            markOpo='X';
    }
    int getMove(Board b){
        int move=9;
        if(b.tc>2){
            move=check(mark,b);
            if(move==9){
                move=check(markOpo,b);
            }
        }
        while((move>8) || (b.s[move]!=' ')){
            move=(int)(Math.random()*9);
        }
        return move;
    }
    int check(char m,Board b){
        int mo;
        mo=checkArray(6,7,8,m,b);

        if(mo!=9)
            return mo;
        mo=checkArray(3,4,5,m,b);

        if(mo!=9)
            return mo;
        mo=checkArray(0,1,2,m,b);

        if(mo!=9)
            return mo;
        mo=checkArray(0,3,6,m,b);

        if(mo!=9)
            return mo;
        mo=checkArray(1,4,7,m,b);

        if(mo!=9)
            return mo;
        mo=checkArray(2,5,8,m,b);

        if(mo!=9)
            return mo;
        mo=checkArray(0,4,8,m,b);

        if(mo!=9)
            return mo;
        mo=checkArray(2,4,6,m,b);

        if(mo!=9)
            return mo;
        else
            return mo;

        }
    int checkArray(int a,int d,int c,char m,Board b){
        if((m==(b.s[a]&b.s[d]))&((b.s[c])==' ')){
            return c;
        }
        else if(((b.s[a]&b.s[c])==m)&&(b.s[d]==' '))
            return d;
        else if(((b.s[d]&b.s[c])==m)&&(b.s[a]==' '))
            return a;
        else
            return 9;
        }

    }

Bugs

if(m==((s[1]&s[4]&s[7])
        |(s[0]&s[3]&s[6])
        |(s[2]&s[5]&s[8])
        |(s[6]&s[7]&s[8])
        |(s[3]&s[4]&s[5])
        |(s[0]&s[1]&s[2])
        |(s[2]&s[4]&s[6])
        |(s[0]&s[4]&s[8]))){

Does this even work? It seems to me that you are trying to test m to each of the above values, but what you are really doing is a binary math operation of & and |. Let me explain...

& "and"s each of the bits in the number, e.g.:

13&19

13:   0 1 1 0 1
17: & 1 0 0 0 1
      0 0 0 0 1

See what happened? 1 and 1 becomes 1, and every other combination becomes 0.

| "or"s the bits:

13:   0 1 1 0 1
17: | 1 0 0 0 1
      1 1 1 0 1

Similar to &, but 0 and 0 result in 0, and every other combination 1.

What is happening is that the char ASCII values are &'d and |'d. Is this really what you want? In this case, you have to do it the tedious way: test each of them to m:

    if (((m == board[1] && m == board[4] && m == board[7])
            || (m == board[0] && m == board[3] && m == board[6])
            || (m == board[2] && m == board[5] && m == board[8])
            || (m == board[6] && m == board[7] && m == board[8])
            || (m == board[3] && m == board[4] && m == board[5])
            || (m == board[0] && m == board[1] && m == board[2])
            || (m == board[2] && m == board[4] && m == board[6])
            || (m == board[0] && m == board[4] && m == board[8]))) {
        return m;
    }

Same here:

if((m==(b.s[a]&b.s[d]))&((b.s[c])==' ')){
    return c;
}
else if(((b.s[a]&b.s[c])==m)&&(b.s[d]==' '))
    return d;
else if(((b.s[d]&b.s[c])==m)&&(b.s[a]==' '))
    return a;
else
    return 9;
}

Becomes:

int checkArray(int a, int d, int c, Mark m, Board b) {
    if ((m == b.getBoard()[a] && m == b.getBoard()[d]) && ((b.getBoard()[c]) == Mark.NONE)) {
        return c;
    } else if ((m == b.getBoard()[a] && m == b.getBoard()[c]) && (b.getBoard()[d] == Mark.NONE)) {
        return d;
    } else if ((m == b.getBoard()[d] && m == b.getBoard()[c]) && (b.getBoard()[a] == Mark.NONE)) {
        return a;
    }
    return 9;
}

Formatting

I can't even read your code. Format it better; in an IDE, you can find a formatting option. In eclipse, it's Source->Format.

If you don't use an IDE, I suggest you install one. Eclipse is available here. Choose the installer, or the "Eclipse IDE for Java Developers" option.

Naming

public class tttmodimproved{

tttmodimproved is not only a bad name, it is also hard to read and understand. Cramming too much information in names is bad. Sure, names like class are bad, but so is thisclassdoesfooandbarwhilealsodoingotherstuff. That's what class-level javadoc comments are for.

As for readability, java class names are in PascalCase, and method and variable names in camelCase.

I suggest TicTacToe, and you can add a short comment about the class at the beginning, like so:

/**
 * ...
 */
public class TicTacToe {
    // ...

Other naming fixes:

cp -> changePlayer
p1 -> player1
p2 -> player2
b -> board
p -> currentPlayer
s -> board
tc: What does this even do?
...

I give up. Let's see what we have so far:

public class TicTacToe {

    static int turn = 0;

    public static void main(String[] args) {
        System.out.println("Let's play TicTacToe");
        char isWin = ' ';
        Board board = new Board();
        Player player1 = new Player('X');
        Player player2 = new Player('O');
        Player currentPlayer;
        for (board.tc = 0; board.tc < 9; board.tc++) {
            currentPlayer = changePlayer(player1, player2);
            board.updateBoard(currentPlayer, currentPlayer.getMove(board));
            isWin = board.checkWin(currentPlayer);
            board.showBoard();
            if (isWin != ' ')
                break;
        }
        if (isWin != ' ')
            System.out.println("congrats! " + isWin + " won the game.");
        else
            System.out.println("tie");

    }

    static Player changePlayer(Player p1, Player p2) {
        if (turn == 0) {
            turn = 1;
            return p1;
        } else if (turn == 1) {
            turn = 0;
            return p2;
        } else
            return p1;
    }

}

class Board {

    char[] board = { ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ' };
    int tc = 0;

    char checkWin(Player p) {
        char m = p.mark;
        if (m == ((board[1] & board[4] & board[7]) | (board[0] & board[3] & board[6]) | (board[2] & board[5] & board[8])
                | (board[6] & board[7] & board[8]) | (board[3] & board[4] & board[5]) | (board[0] & board[1] & board[2])
                | (board[2] & board[4] & board[6]) | (board[0] & board[4] & board[8]))) {
            return m;
        }
        return (' ');
    }

    void showBoard() {
        System.out.println("");
        System.out.println(" " + board[6] + " | " + board[7] + " | " + board[8] + " ");
        System.out.println("---|---|---");
        System.out.println(" " + board[3] + " | " + board[4] + " | " + board[5] + " ");
        System.out.println("---|---|---");
        System.out.println(" " + board[0] + " | " + board[1] + " | " + board[2] + " ");
        System.out.println("");
    }

    void updateBoard(Player player, int move) {
        board[move] = player.mark;
    }

}

class Player {
    char mark = ' ', markOpo = ' ';

    Player(char mark) {
        this.mark = mark;
        System.out.println("player created with mark: " + this.mark);
        if (this.mark == 'X') {
            markOpo = 'O';
        } else
            markOpo = 'X';
    }

    int getMove(Board b) {
        int move = 9;
        if (b.tc > 2) {
            move = check(mark, b);
            if (move == 9) {
                move = check(markOpo, b);
            }
        }
        while ((move > 8) || (b.board[move] != ' ')) {
            move = (int) (Math.random() * 9);
        }
        return move;
    }

    int check(char m, Board b) {
        int mo;
        mo = checkArray(6, 7, 8, m, b);

        if (mo != 9)
            return mo;
        mo = checkArray(3, 4, 5, m, b);

        if (mo != 9)
            return mo;
        mo = checkArray(0, 1, 2, m, b);

        if (mo != 9)
            return mo;
        mo = checkArray(0, 3, 6, m, b);

        if (mo != 9)
            return mo;
        mo = checkArray(1, 4, 7, m, b);

        if (mo != 9)
            return mo;
        mo = checkArray(2, 5, 8, m, b);

        if (mo != 9)
            return mo;
        mo = checkArray(0, 4, 8, m, b);

        if (mo != 9)
            return mo;
        mo = checkArray(2, 4, 6, m, b);

        if (mo != 9)
            return mo;
        else
            return mo;

    }

    int checkArray(int a, int d, int c, char m, Board b) {
        if ((m == (b.board[a] & b.board[d])) & ((b.board[c]) == ' ')) {
            return c;
        } else if (((b.board[a] & b.board[c]) == m) && (b.board[d] == ' '))
            return d;
        else if (((b.board[d] & b.board[c]) == m) && (b.board[a] == ' '))
            return a;
        else
            return 9;
    }

}

Now let's do some actual reviewing...

OOP

Java is an Object Oriented Programming (OOP) language, which means it's based on objects. You did a good job trying, but if you leave a default constructor and let the calling code do the work, it's not OOP at all. Let's do some serious redesigning:

public class TicTacToe {

    public static void main(String[] args) {
        System.out.println("Let's play TicTacToe");
        Mark win = Mark.NONE;
        Board board = new Board();
        Player player1 = new Player(Mark.X);
        Player player2 = new Player(Mark.O);
        Player currentPlayer = null;
        for (int tc = 0; tc < 9; tc++) {
            currentPlayer = switchPlayer(currentPlayer, player1, player2);
            board.updateBoard(currentPlayer, currentPlayer.getMove(board));
            win = board.checkWin(currentPlayer);
            board.showBoard();
            if (win != Mark.NONE) {
                break;
            }
        }
        if (win != Mark.NONE)
            System.out.println("congrats! " + win + " won the game.");
        else
            System.out.println("tie");

    }

    private static Player switchPlayer(Player currentPlayer, Player player1, Player player2) {
        if (currentPlayer == player1) {
            return player2;
        }
        return player1;
    }

}

class Board {

    private Mark[] board = { Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE };
    private int tc = 0;

    public Board() {

    }

    public Mark[] getBoard() {
        return board;
    }

    public int getTC() {
        return tc;
    }

    public void setTC(int tc) {
        this.tc = tc;
    }

    public Mark checkWin(Player p) {
        Mark m = p.getMark();
        if (((m == board[1] && m == board[4] && m == board[7]) || (m == board[0] && m == board[3] && m == board[6])
                || (m == board[2] && m == board[5] && m == board[8])
                || (m == board[6] && m == board[7] && m == board[8])
                || (m == board[3] && m == board[4] && m == board[5])
                || (m == board[0] && m == board[1] && m == board[2])
                || (m == board[2] && m == board[4] && m == board[6])
                || (m == board[0] && m == board[4] && m == board[8]))) {
            return m;
        }
        return Mark.NONE;
    }

    public void showBoard() {
        System.out.println("");
        System.out.println(" " + board[6] + " | " + board[7] + " | " + board[8] + " ");
        System.out.println("---|---|---");
        System.out.println(" " + board[3] + " | " + board[4] + " | " + board[5] + " ");
        System.out.println("---|---|---");
        System.out.println(" " + board[0] + " | " + board[1] + " | " + board[2] + " ");
        System.out.println("");
    }

    public void updateBoard(Player player, int move) {
        board[move] = player.getMark();
    }

}

class Player {

    private Mark mark = Mark.NONE;
    private Mark markOpo = Mark.NONE;

    public Player(Mark mark) {
        this.mark = mark;
        System.out.println("player created with mark: " + this.mark);
        if (this.mark == Mark.X) {
            markOpo = Mark.O;
        } else {
            markOpo = Mark.X;
        }
    }

    public Mark getMark() {
        return mark;
    }

    public int getMove(Board b) {
        int move = 9;
        if (b.getTC() > 2) {
            move = check(mark, b);
            if (move == 9) {
                move = check(markOpo, b);
            }
        }
        while (move > 8 || b.getBoard()[move] != Mark.NONE) {
            move = (int) (Math.random() * 9);
        }
        return move;
    }

    int check(Mark mark, Board board) {
        int mo;
        mo = checkArray(6, 7, 8, mark, board);
        if (mo != 9)
            return mo;
        mo = checkArray(3, 4, 5, mark, board);

        if (mo != 9)
            return mo;
        mo = checkArray(0, 1, 2, mark, board);

        if (mo != 9)
            return mo;
        mo = checkArray(0, 3, 6, mark, board);

        if (mo != 9)
            return mo;
        mo = checkArray(1, 4, 7, mark, board);

        if (mo != 9)
            return mo;
        mo = checkArray(2, 5, 8, mark, board);

        if (mo != 9)
            return mo;
        mo = checkArray(0, 4, 8, mark, board);

        if (mo != 9)
            return mo;
        mo = checkArray(2, 4, 6, mark, board);

        if (mo != 9)
            return mo;
        else
            return mo;

    }

    private int checkArray(int a, int d, int c, Mark m, Board b) {
        if ((m == b.getBoard()[a] && m == b.getBoard()[d]) && ((b.getBoard()[c]) == Mark.NONE)) {
            return c;
        } else if ((m == b.getBoard()[a] && m == b.getBoard()[c]) && (b.getBoard()[d] == Mark.NONE)) {
            return d;
        } else if ((m == b.getBoard()[d] && m == b.getBoard()[c]) && (b.getBoard()[a] == Mark.NONE)) {
            return a;
        }
        return 9;
    }

}

enum Mark {
    NONE, X, O;

    @Override
    public String toString() {
        switch (this) {
        case NONE:
            return " ";
        case O:
            return "O";
        case X:
            return "X";
        default:
            return "";
        }
    }
}

A little explanation in the changes I made:

char is now Mark. It's more OOP that way, as char don't exactly represent a mark that a player makes.
default level methods are now either public or private. OOP people don't like default privacy methods.
turn variable in the Main class is now gone.

Now, moving to other changes:

The check method can be improved.

    if (mo != 9)
        return mo;
    else
        return mo;

This statement is redundant. It says, if mo is not 9, then return the value of mo. Otherwise, return the value of mo.

That can easily be:

    return mo;

    mo = checkArray(6, 7, 8, mark, board);
    if (mo != 9)
        return mo;
    mo = checkArray(3, 4, 5, mark, board);

    if (mo != 9)
        return mo;
    mo = checkArray(0, 1, 2, mark, board);

    if (mo != 9)
        return mo;
    mo = checkArray(0, 3, 6, mark, board);

    if (mo != 9)
        return mo;
    mo = checkArray(1, 4, 7, mark, board);

    if (mo != 9)
        return mo;
    mo = checkArray(2, 5, 8, mark, board);

    if (mo != 9)
        return mo;
    mo = checkArray(0, 4, 8, mark, board);

    if (mo != 9)
        return mo;
    mo = checkArray(2, 4, 6, mark, board);

A lot of repetition here. Use loops:

int check(Mark mark, Board board) {
    int mo;
    // rows
    for (int i = 0; i < 9; i += 3) {
        mo = checkArray(i, i + 1, i + 2, mark, board);
        if (mo != 9) {
            return mo;
        }
    }
    // columns
    for (int i = 0; i < 3; i++) {
        mo = checkArray(i, i + 3, i + 6, mark, board);
        if (mo != 9) {
            return mo;
        }
    }
    // diagonals
    mo = checkArray(0, 4, 8, mark, board);
    if (mo != 9) {
        return mo;
    }

    mo = checkArray(2, 4, 6, mark, board);
    return mo;

}

Final Code:

public class TicTacToe {

    public static void main(String[] args) {
        System.out.println("Let's play TicTacToe");
        Mark win = Mark.NONE;
        Board board = new Board();
        Player player1 = new Player(Mark.X);
        Player player2 = new Player(Mark.O);
        Player currentPlayer = null;
        for (int tc = 0; tc < 9; tc++) {
            currentPlayer = switchPlayer(currentPlayer, player1, player2);
            board.updateBoard(currentPlayer, currentPlayer.getMove(board));
            win = board.checkWin(currentPlayer);
            board.showBoard();
            if (win != Mark.NONE) {
                break;
            }
        }
        if (win != Mark.NONE)
            System.out.println("congrats! " + win + " won the game.");
        else
            System.out.println("tie");

    }

    private static Player switchPlayer(Player currentPlayer, Player player1, Player player2) {
        if (currentPlayer == player1) {
            return player2;
        }
        return player1;
    }

}

class Board {

    private Mark[] board = { Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE, Mark.NONE };
    private int tc = 0;

    public Board() {

    }

    public Mark[] getBoard() {
        return board;
    }

    public int getTC() {
        return tc;
    }

    public void setTC(int tc) {
        this.tc = tc;
    }

    public Mark checkWin(Player p) {
        Mark m = p.getMark();
        if (((m == board[1] && m == board[4] && m == board[7]) || (m == board[0] && m == board[3] && m == board[6])
                || (m == board[2] && m == board[5] && m == board[8])
                || (m == board[6] && m == board[7] && m == board[8])
                || (m == board[3] && m == board[4] && m == board[5])
                || (m == board[0] && m == board[1] && m == board[2])
                || (m == board[2] && m == board[4] && m == board[6])
                || (m == board[0] && m == board[4] && m == board[8]))) {
            return m;
        }
        return Mark.NONE;
    }

    public void showBoard() {
        System.out.println("");
        System.out.println(" " + board[6] + " | " + board[7] + " | " + board[8] + " ");
        System.out.println("---|---|---");
        System.out.println(" " + board[3] + " | " + board[4] + " | " + board[5] + " ");
        System.out.println("---|---|---");
        System.out.println(" " + board[0] + " | " + board[1] + " | " + board[2] + " ");
        System.out.println("");
    }

    public void updateBoard(Player player, int move) {
        board[move] = player.getMark();
    }

}

class Player {

    private Mark mark = Mark.NONE;
    private Mark markOpo = Mark.NONE;

    public Player(Mark mark) {
        this.mark = mark;
        System.out.println("player created with mark: " + this.mark);
        if (this.mark == Mark.X) {
            markOpo = Mark.O;
        } else {
            markOpo = Mark.X;
        }
    }

    public Mark getMark() {
        return mark;
    }

    public int getMove(Board board) {
        int move = 9;
        if (board.getTC() > 2) {
            move = check(mark, board);
            if (move == 9) {
                move = check(markOpo, board);
            }
        }
        while (move > 8 || board.getBoard()[move] != Mark.NONE) {
            move = (int) (Math.random() * 9);
        }
        return move;
    }

    int check(Mark mark, Board board) {
        int mo;
        // rows
        for (int i = 0; i < 9; i += 3) {
            mo = checkArray(i, i + 1, i + 2, mark, board);
            if (mo != 9) {
                return mo;
            }
        }
        // columns
        for (int i = 0; i < 3; i++) {
            mo = checkArray(i, i + 3, i + 6, mark, board);
            if (mo != 9) {
                return mo;
            }
        }
        // diagonals
        mo = checkArray(0, 4, 8, mark, board);
        if (mo != 9) {
            return mo;
        }

        mo = checkArray(2, 4, 6, mark, board);
        return mo;

    }

    private int checkArray(int a, int d, int c, Mark m, Board b) {
        if ((m == b.getBoard()[a] && m == b.getBoard()[d]) && ((b.getBoard()[c]) == Mark.NONE)) {
            return c;
        } else if ((m == b.getBoard()[a] && m == b.getBoard()[c]) && (b.getBoard()[d] == Mark.NONE)) {
            return d;
        } else if ((m == b.getBoard()[d] && m == b.getBoard()[c]) && (b.getBoard()[a] == Mark.NONE)) {
            return a;
        }
        return 9;
    }

}

enum Mark {
    NONE, X, O;

    @Override
    public String toString() {
        switch (this) {
        case NONE:
            return " ";
        case O:
            return "O";
        case X:
            return "X";
        default:
            return "";
        }
    }
}


Comparing two images I have some code in Go that is attempting to compare two images and determine how different they are. The method is fairly crude, but it is fast enough. Basically it just compares both images pixel by pixel and for each pixel it determines how different they are by comparing the RGBA values separately and using the absolute value of the difference (summed across the 4 channels) as the difference value for the current pixel. It sums together all these difference values and uses that number as the final result (that is then printed).

Does anyone have any comment on how this can be improved? Specifically for accuracy, as I mentioned, performance is "good enough" for now, though I'm not opposed to comments on efficiency as well.

package main

import (
    "os"
    "log"
    "image"
    "image/color"
    "math"
    "fmt"

    _ "image/png"
)

func main () {
    // open the target file
    target_file, err := os.Open("/home/gregg/workspace/Fueled/imagefilters/www/test_images/nashville.png")
    if (err != nil) {
        log.Fatal(err)
    }
    defer target_file.Close()

    // open the source file
    source_file, err := os.Open("/home/gregg/workspace/Fueled/imagefilters/www/test_images/out.png")
    if (err != nil) {
        log.Fatal(err)
    }
    defer source_file.Close()

    // decode the target file
    target, _, err := image.Decode(target_file)
    if (err != nil) {
        log.Fatal(err)
    }

    // decode the source file
    source, _, err := image.Decode(source_file)
    if (err != nil) {
        log.Fatal(err)
    }

    // check to make sure Bounds match
    target_bounds := target.Bounds()
    source_bounds := source.Bounds()
    if (!boundsMatch(target_bounds, source_bounds)) {
        log.Fatal("Image sizes don't match!")
    }

    var diff int64
    for y := target_bounds.Min.Y; y < target_bounds.Max.Y; y++ {
        for x := target_bounds.Min.X; x < target_bounds.Max.X; x++ {
            diff += compareColor(target.At(x, y), source.At(x, y))
        }
    }
    fmt.Printf("%d\n", diff)
}

func compareColor(a, b color.Color) (diff int64) {
    r1, g1, b1, a1 := a.RGBA()
    r2, g2, b2, a2 := b.RGBA()

    diff += int64(math.Abs(float64(r1-r2)))
    diff += int64(math.Abs(float64(g1-g2)))
    diff += int64(math.Abs(float64(b1-b2)))
    diff += int64(math.Abs(float64(a1-a2)))
    return diff
}

func boundsMatch(a, b image.Rectangle) bool {
    return a.Min.X == b.Min.X && a.Min.Y == b.Min.Y && a.Max.X == b.Max.X && a.Max.Y == b.Max.Y
}

Though I didn't check the whole program and, according to you, performance is good enough, I do have one performance related suggestion:

avoid int64(math.Abs(float64(var1-var2))). Casting between int and float types is not free. Just read about Two's complement for ints and how floats are represented in the IEEE 754 standard. That conversion is not trivial!

As Go (as far as I know) doesn't have an abs method for int-like types, this may provide inspiration on how to calculate the abs for signed ints.


Analog watch simulation I wrote an analog watch simulation in C# and Monogame as exercise.

The application consists of a background texture. On top of that I have the hour-, minute- and second hand.

The program gets its time from DateTime.Now.

The second hand rotates by multiplying the seconds with 1/60 of a total lap in radians.

It's the same with the minute hand and hour hand.

Source code:

class Clock
{
    int hour;
    int minutes;
    int seconds;

    // Textures
    public Texture2D CaseBack;
    public Texture2D HourHand;
    public Texture2D MinuteHand;
    public Texture2D SecondHand;

    Vector2 center = new Vector2(63, 63); // Center of watch

    // Gets called 60 times/second
    public void Update(GameTime gameTime)
    {
        hour = DateTime.Now.Hour;
        minutes = DateTime.Now.Minute;
        seconds = DateTime.Now.Second;
    }

    // Gets called 60 times/second
    public void Draw(SpriteBatch spriteBatch)
    {
        // Background/Case back
        spriteBatch.Draw(CaseBack, Vector2.Zero, Color.White);

        // Second hand
        spriteBatch.Draw(SecondHand, center, null, null, new Vector2(3, 50),
            ((float)seconds*(float)Math.PI*2 / 60),
            // This is the rotation. Works by multiplying Seconds with a 1/60 of a "circle's angle".
            null, Color.White, SpriteEffects.None, 0);

        // Minute hand - basically the same, but with minutes
        spriteBatch.Draw(MinuteHand, center, null, null, new Vector2(3, 40),
            ((float)minutes * (float)Math.PI * 2 / 60),
            null, Color.White, SpriteEffects.None, 0);

        // Hour hand - same here
        spriteBatch.Draw(HourHand, center, null,null, new Vector2(4, 30),
            ((float)hour * (float)Math.PI * 2 / 12),
            null, Color.White, SpriteEffects.None, 0);
    }

Don't repeat yourself

The step of drawing the hour, minute, second hands are all very similar.
It would be good to use a helper function:

public void Draw(SpriteBatch spriteBatch, Texture2D hand, int value, Vector2 vector2, float coef)
{
    spriteBatch.Draw(hand, center, null, null, vector2,
        value * Math.PI * coef,
        null, Color.White, SpriteEffects.None, 0);
}

Visibility

Since the hour, minutes, seconds should not be visible outside the class,
it would be good to make them explicitly private.
(Thanks @Maxim for the clarification that the default access modifier is actually private.)


React props children with List and Spinner Loader I have a custom component with a List. I need to know if this way is the best for show a Spinner while the fetch is working.

I'm using child props for that, but I don't know if this way is the correct.

import React, { Component, PropTypes } from 'react';
import demoData from './demodata';
// Other import stuff...

export default class HomeScreen extends Component {
  // Constructor component
  constructor(props) {
    super(props);
    const ds = new ListView.DataSource({
      rowHasChanged: (r1, r2) => r1 !== r2,
      sectionHeaderHasChanged: (s1, s2) => s1 !== s2
    });

    this.state = {
      dataSource: ds.cloneWithRows(demoData),
      loading: true
    };

    // Service for fetching data
    this.service = new ServiceApp();
  }

  componentDidMount() {
    this
      .fetchData();

  }

 // Simulate fetchData
 fetchData() {
      const self = this;
      setTimeout(() => {
         self.setState({
            loading: false
         });
      }, 4000);
  }

  render() {
    const component = this.state.loading ? <Spinner /> : (<ListView
      style={styles.container}
      dataSource={this.state.dataSource}
      renderRow={data => <Row {...data} />} />);

    return (
        <Content>
          {component}
        </Content>
    );
  }
}

The way I do it is to have both the Spinner and the components in the render and pass it the loading prop and inside the Spinner component it will render the spinner if loading is set to true and return null if its set to false. That way you're not re-rendering the whole listview when you toggle it on and off.

this.state = {
  loading: false,
} //in the contructor

componentDidMount() {
this.setState({ loading: true }, () => this.fetchData());
}

// Simulate fetchData
fetchData = () => {
  //const self = this; you don't need this line
  setTimeout(() => {
     this.setState({
        loading: false
     });
  }, 4000);        //after you receive the response, you set it back to false
}

render() {

  return (
    <Content>
      <Spinner visible={this.state.loading} />
      <ListView
        style={styles.container}
        dataSource={this.state.dataSource}
        renderRow={data => <Row {...data} />} />
    </Content>
  );
}


Searching a Word Document from Excel Today I created a script in Excel that migrates data from a specific Word document and copies a portion of it to a cell in Excel (the date to be specific).

The file input is

  Last name, First Name, Grade - (Rank) (Completed On YYYY-MM-DD HH:mm:ss)

and each person is on a new "paragraph" (aka line) in Word.

Sub SearchTextFile()
Dim i As Integer
Dim currentCellNumberLname As String
Dim currentCellNumberFname As String
Dim currentCellData As String
Dim firstPlusLastName As String
Dim filePath As String
filePath = "C:\Users\<user>\Desktop\CyberAwareness.docx"

For i = 2 To 4
    currentCellNumberLname = "C" & i
    currentCellNumberFname = "D" & i
    currentCellToAdd = "L" & i
    ActiveSheet.Range(currentCellToAdd).Activate
    ActiveWindow.ScrollRow = ActiveCell.Row
    firstPlusLastName = Range(currentCellNumberLname).Value & ", " & Range(currentCellNumberFname).Value
    Range(currentCellToAdd) = SearchWordDoc(filePath, firstPlusLastName)
Next
End Sub

'Searches word file for name, finds the associated paragraph, and returns the date'
Function SearchWordDoc(strPath, strName)
Set objword = CreateObject("word.application")
Set a = objword.documents.Open(strPath)

For i = 1 To a.Paragraphs.Count
    If InStr(a.Paragraphs(i).Range.Text, strName) <> 0 Then
        SearchWordDoc = Left(Right(a.Paragraphs(i).Range.Text, 22), 11)
    End If
Next i
a.Close
objword.Quit
'    objword.Visible = False'
Set objword = Nothing
End Function

I have about 300 personnel that need information entered into this document and about 8 different columns I need to populate. The script works surprisingly well, however, it is pretty slow and takes ~10 minutes for one column. I'm fine with it since it gets the job done, but I'm curious if there is any way to streamline it further and possibly add error checking (for name mismatches).

General Notes

First - In addition to replacing the magic numbers, I'd put your file path into a constant at the top of the module for easier maintenance:

Private Const WORD_DOCUMENT_LOCATION As String = "C:\Users\<user>\Desktop\CyberAwareness.docx"

Second - Get a reference to the Worksheet object at the start of the function instead of relying on calls to ActiveSheet.  Nothing will make the wheels come off of a long function faster than the user doing something that changes the active Workbook or Worksheet in the middle of your execution.

Dim sheet As Worksheet
Set sheet = ActiveSheet

Third - Using Range addressing C2 or L3 is not only harder to read, it is less efficient as well. Not only do you have to concatenate strings in order to build the addresses, Excel just has to convert them back into numeric indexes. Unless you have a really good reason not to do so, using .Cell(row, column) is almost always better and is much easier to use in loops.

'Instead of this...
sheet.Range("A" & i).Value = foobar
'...do this:
sheet.Cells(i, 1).Value = foobar

Fourth - Get into the habit of declaring the scope of your Subs and Functions to make sure that you are only exposing the ones that you intend to. If you leave these off, they will default to public and start showing up in your function list in the Workbook and be available for auto-complete. If it isn't something you would want to show up in a cell, i.e. =SearchWordDoc("this raises","an error"), declare it as Private.

Finally - Declare your function return types. It isn't clear what the code below returns, and the comment makes it worse - it doesn't return a Date at all, it returns a String.

'Searches word file for name, finds the associated paragraph, and returns the date'
Function SearchWordDoc(strPath, strName)

Taken in conjunction with the item above, the function declaration should really be something like this:

Private Function SearchWordDoc(filepath, name) As String

Performance

As @RubberDuck noted, the biggest performance boost you're going to get is from not repeatedly opening and closing the Word document. Most of the other suggestions are spot on, so I'll limit this answer to the performance aspect of the code.

Based on your existing code, it is apparent that there is only one line per person in your Word document and all of the lines have (or should have) the same structure.  What you are essentially doing is performing a lookup on a Word file based on a key.  VBA has an object that is designed to perform key lookups that are orders of magnitude faster than Word's .Find method - Scripting.Dictionary. All you need to do is parse the document first as text to build your object.  For example (with a reference to Microsoft Scripting Runtime):

Private Function LoadWordRecords(filepath As String) As Scripting.Dictionary

    Dim host As New Word.Application
    Dim doc As Word.Document
    Dim lines() As String

    Set doc = host.Documents.Open(filepath)
    'Take the whole document in one shot, and read to an array of paragraphs...
    lines = Split(doc.Content.text, vbCr)
    '...and dispense with Word.
    doc.Close
    host.Quit

    'Then do whatever you need to do to parse the text into a useful structure:
    Dim output As New Scripting.Dictionary
    Dim items() As String
    Dim i As Integer

    'Parse each line in the Word document to extract a key for the line.
    For i = 0 To UBound(lines)
        'Add whatever you need to validate the paragraph you're parsing.
        'Test the input line to see if it contains a comma.
        If (InStr(1, lines(i), ",") > 0) Then
            'Split into an array from the comma delimited string.
            items = Split(lines(i), ",")
            'Add the resulting item to the Dictionary with the key defined
            'as the first 2 elements and the value as the remainder of the line.
            Call output.Add(Trim$(items(0)) & ", " & Trim$(items(1)), items(2))
        End If
        'Or whatever is convenient.
    Next i

    Set LoadWordRecords = output

End Function

Then, all you have to do in the calling code is grab the Dictionary from your parsing function and grab the lines as needed based on the keys:

Public Sub SearchTextFile()

    Dim sheet As Worksheet
    Set sheet = ActiveSheet

    Dim i As Integer
    Dim currentCellNumberLname As String
    Dim currentCellNumberFname As String
    Dim currentCellData As String
    Dim firstPlusLastName As String

    'Load your lookup object:
    Dim entries As Scripting.Dictionary
    Set entries = LoadWordRecords(WORD_DOCUMENT_LOCATION)

    For i = 2 To 4
        currentCellNumberLname = "C" & i
        currentCellNumberFname = "D" & i
        currentCellToAdd = "L" & i
        ActiveSheet.Range(currentCellToAdd).Activate
        ActiveWindow.ScrollRow = ActiveCell.row
        firstPlusLastName = sheet.Range(currentCellNumberLname).Value & ", " & sheet.Range(currentCellNumberFname).Value

        'Retrieve lines by keys for parsing:
        sheet.Range(currentCellToAdd) = Left(Right(entries(firstPlusLastName), 22), 11)
    Next

End Sub

My rule of thumb is that Windows COM calls are generally expensive, and are best avoided if alternatives exist. This should be finishing on the order of seconds not minutes, and the majority of the processing time should be coming from simply opening the Word document.


Calculate total water drunk in a day I would like to know how I might improve my code?
I just started coding and liking it a few days ago so I am quite new to this whole world...

The objective is to calculate the amount of water one has drunk in a day. Depending on the user's input, the app tells him or her if the amount of water drunk is great or not.

Now I would like know how I might improve this whole code?
And I'd be grateful for some advice (what I could improve etc)...

package com.ahmed.everything;
import java.util.*;
import java.util.concurrent.TimeUnit;

public class DrinkingWater {
    public static void main(String [] args) throws Exception {
    Scanner sc = new Scanner(System.in);

    System.out.print("Amount of water your bottle can hold: ");
    // entering amount of water bottle can hold
    double litre = sc.nextDouble();
    System.out.print("How many bottles have you drunk today? : ");
    // amount of bottles for today
    double bottles = sc.nextDouble();
    // calculating water drunk
    double total = (bottles * litre);

    // result
    if (bottles == 1) {
    System.out.print("You've only drunk one bottle today! :(" + "\n");
    }
    else {
        System.out.println("Today, you have drunk " + bottles + " bottles. Which equals to " + total + " liters of water.");
    }

    // advice
    if (total < 4) {
        System.out.println("You did not drink enough water, drink at least 4 liters to stay healthy!");
    }

    else if (total >= 4) {
        System.out.println("Good job! You've drunk enough water today! :)");
    }
    else;

    //short time span
    TimeUnit.SECONDS.sleep(1);
    System.out.print(".");
    TimeUnit.SECONDS.sleep(1);
    System.out.print(".");
    TimeUnit.SECONDS.sleep(1);
    System.out.println(".");
    TimeUnit.SECONDS.sleep(1);

    System.out.println("Did you like our little app? Please review it from 1-5. 1 being considerer as very good and 5 as very bad.");
    int note = sc.nextInt();
    if (note <= 4 || note >= 1) {
        System.out.println("Thanks for your review.");
    }else {
        System.out.println("That's better, we'll do our best the next time");
    }

    }
}

Check your if expression in review part:

if (note <= 4 || note >= 1) {
    System.out.println("Thanks for your review.");
}else {
    System.out.println("That's better, we'll do our best the next time");
}

This will always print "Thanks for your review" since whatever the value of note will be always true either for <= 4 or >= 1.

Using an IDE (for example IntelliJ, netbeans, eclipse) helps you mitigate this kind of problems, but it's still better to always review your code.

Confusing result and advise:

I'm not sure if the application is happy that I drank enough water or sad because I only drank 1 bottle.

As stated in your question:

  The objective is to calculate the amount of water one has drunk in a
  day. Depending on the user's input, the app tells him or her if the
  amount of water drunk is great or not.

Then I think you don't need to tell them how many bottles they drink or at least remove the sad face to avoid confusion.

User input problems:

Case 1: What if the user input a letter instead of number? This will happen:

Case 2: What if the user input a negative number? Then this will happen:

As you can see in case 1, this will break your program. Although in case 2 the program will not crash, I think it's impossible to drink negative amount of water.

To solve this kind of issue, you can use try/catch expression. Something like this:

double litre = 0;
boolean isInvalidInput = false;
do {
    try {
        System.out.print("Amount of water your bottle can hold: ");
        // entering amount of water bottle can hold
        litre = sc.nextDouble();
        isInvalidInput = false;
    } catch (InputMismatchException e) {
        isInvalidInput = true;
    }
    sc.nextLine();
} while(isInvalidInput || litre < 0);

If you use the code above, this will be the output:

As you can see, the program will ask the same question repeatedly until the user enters a correct input


Check if relations exists for a parent node before deleting it so that we don't have any orphan child node Here I am trying to delete a parent node but only if it doesn't have any child node. Please review this code.

PostController.php

/**
 * Delete the given post.
 *
 * @param  int $post_id
 * @return void
 *
 * @throws \App\Exceptions\RelationExistsException
 */
public function deletePost($post_id)
{
    $relations = ['tags', 'comments'];

    $hasRelations = $this->postRepo->hasRelations($post_id, $relations);

    if ($hasRelations) {
        throw new RelationExistsException($relations);
    }

    $this->postRepo->delete($post_id);
}

RepositoryTrait.php

/**
 * Check if any relation exists.
 *
 * @param  int  $id
 * @param  array  $relations
 * @return bool
 */
public function hasRelations($id, array $relations)
{
    if (count($relations) == 0) {
        throw new \Exception('No relation is provided.');
    }

    $modelName = $this->model;

    $query = $modelName::where('id', $id);

    $query->where(function($q) use($relations) {
        foreach ($relations as $relation) {
            $q->orHas($relation);
        }
    });

    return $query->exists();
}

RelationExistsException.php

namespace App\Exceptions;

use Exception;

class RelationExistsException extends Exception
{
    /**
    * Create a new exception instance.
    *
    * @param array $relations
    * @return void
    */
    function __construct($relations)
    {
        parent::__construct(sprintf(
            'Cannot delete because there exists relations - %s.', implode(', ', $relations))
        );
    }
}

I have one more issue with the following code:

$modelName = $this->model; // contains '\App\Models\Post'

$query = $modelName::where('id', $id);

I have to save the $this->model in $modelName to use it with the where function. Is there a better solution for it. I have tried {}, () but nothing worked.

GitHub Gist for above code

Some thoughts below:

I would encourage you to think about validating parameters more thoroughly on your public methods.  For example, in deletePosts(), you do nothing to validate that you even have an integer value (or whatever) to work with.  You do it in some cases such as type-hinting for array and validating non-empty array on hasRelations(), but it is not consistent.  What if non-array is passed to your exception constructor? What if non-integer (or whatever) is passed to deletePosts()?

You may not think it matters now because you are currently working in this application area and understand where all the calls are made against these methods, but think about in the future when you try to leverage these classes in new ways.  The call patterns may be different.  If, for example, you introduce a buggy use case that passes a non-array to your exception class, you want that class to complain loudly, rather than silently fail so that you can focus your debugging efforts more quickly on the problem code.

$relations = ['tags', 'comments'];

Why is this hard-coded here?  I would think this should, at a minimum, be a property on the class, if not derived from configuration.

$hasRelations = $this->postRepo->hasRelations($post_id, $relations);

Consider placing this code in a try-catch block since hasRelations() can throw. I know that since $relations is hard-coded here that we would never expect to get into a state where that exception is thrown, however I think it best practice to always use try-catch block in such a case so that as someone working in this code, you have quick understanding of how methods you are calling might perform.  Who knows, maybe the implementation of RepositoryTrait changes at some point and throws exceptions for other reasons besides the passed relations parameter.

throw new \Exception('No relation is provided.');

Consider throwing InvalidArgumentException if you want to be more specific here.  I actually find it a bit odd that you use custom exception types in the code calling this but not here.  It might call into question how you are using custom exceptions throughout the application.

Your custom exception seems to have a very limited use case. It really only exists to format the message string from a passed array, functionality that might rightfully live in the code where the exception is thrown not within the logic of the exception.

Is this message even meaningful to caller as is?  You can't tell what kind of relationship exists amongst the types provided, nor the specific ID's of the relationships that exist, so there is questionable value in preparing this specific message string vs. just a simple 'Cannot delete because this post has relations' message in the exception. You could easily provide this message in context of where the exception is thrown totally eliminating the need to override the constructor (or maybe even this class altogether).

Is this class going to be used elsewhere in your application?  If not, should it even exist (vs. using other exception types)? If so, are you always going to want to pass this class an array of relation types as parameter to format into message string?

I actually question whether you should even be throwing an exception here at all if a relationship exists.  Since you are specifically building this functionality, my guess is that you are expecting the application to handle deletion requests against posts that have relationships as part of normal operation of the application.  If so, should this really be an exception or just an alternate code path that needs to be followed to handle this condition?  Only if you truly never expect the application to be put into this state would it make sense to have this code throw an exception.

To your question about how to call model dynamically while avoiding having to set $modelName, you could use something like:

call_user_func($this->model . '::where', 'id', $id);

but I honestly find what you have in place to be easier to read, and I would not be concerned about the cost of having the additional variable in memory (that seems like a micro-optimization type of concern).


Beep-beep I'm a car factory I have implemented to following generic factory in C++11 using smart pointers and I would like to get some feedback about it.

(Please note that I can't use C++14)

Due to company policies I have to change the names of the business objects.

class Car{};
class Taxi : public Car{};
class Police : public Car{};

class CarFactory {

public:
    enum CarName {TaxiName, PoliceName};

    std::unique_ptr<Car> getCar(CarName sa) {
        if (d_carMap.find(sa) == d_carMap.end()) {
            d_carMap[sa] = std::move(createCar(sa));
        }
        return std::move(d_carMap[sa]);
    }

private:
    std::unordered_map<CarName,
                       std::unique_ptr<Car>,
                       std::hash<int>> d_carMap; //hash is for enum

    std::unique_ptr<Car> createCar(CarName sa) {

        if (sa == CarName::TaxiName){
          return std::unique_ptr<Car>(new Taxi());
        } else if (sa == CarName::PoliceName) {
          return std::unique_ptr<Car>(new Police());
        }

        return nullptr; //no car found for provided argument.
    }

};

Which of the moves are actually needed here?
What happens to the unique_ptrs, if this factory is used in a multi threading environment?
Is std::unique_ptr<Car>(new Taxi()); really the best way of instantiating a smart pointer in C++11 (which doesn't have make_unique)?
Should the enum be a class, even if I can't use std::hash on it?
Where would you define the enum? In a separate header file? In the same
header, outside of the class (but in a namespace)?
How should the unit test look like?

I wrote the following test which actually fails on the indicated line:

TEST(CarFactoryTest, retrieveTaxi) {

    CarFactory sf;

    auto Car = sf.getCar(CarFactory::CarName::TaxiName);
    if (!Car) {
        FAIL();
    }

    if (dynamic_cast<Taxi*>(Car.get()) == nullptr) {
        FAIL();
    }

    //this time get it from the map
    Car = sf.getCar(CarFactory::CarName::TaxiName);
    if (!Car) {
        FAIL(); //Test fails here!
    }

    if (dynamic_cast<Taxi *>(Car.get()) == nullptr) {
        FAIL();
    }
}

1) Which of the moves are actually needed here?

Since there are only two moves, let's have a look at them:

std::unique_ptr<Car> getCar(CarName sa) {
    if (d_carMap.find(sa) == d_carMap.end()) {
        d_carMap[sa] = std::move(createCar(sa)); // (1)
    }
    return std::move(d_carMap[sa]);              // (2)
}

Only the latter is necessary. In (1), the expression createCar(sa) is a prvalue expression and therefore a rvalue. In (2), d_carMap[sa] is a lvalue expression, since operator[] returns a lvalue reference.

Or: if you try to move something that has a name, or if the function returns T&, you need std::move.

  2) What happens to the unique_ptrs, if this factory is used in a multi threading environment?

Undefined behaviour. There's a race condition if one uses std::move on the same unique_ptr twice: the move assignment gets evaluated half-through twice, you end up with two unique_ptr owning the same memory, and two calls to delete. Note that the std::unorded_map isn't thread-safe either.

  3) Is std::unique_ptr(new Taxi()); really the best way of instantiating a smart pointer in C++11 (which doesn't have make_unique)

No. Use make_unique, but make sure to put it in your own namespace, not std. That way, you can simply replace your version with the C++14 one later.

  4) Should the enum be a class, even if I can't use std::hash on it?

That doesn't really make sense. Either you have an enumeration, or you don't. Unless you mean a scoped enumeration (enum class). And yes, scoped enums are usually better. But that depends on 5).

And you can use std::hash on a scoped enum, even in C++11, since you're allowed to specialize std::hash for your type. It's one of the exceptions where you may mingle in the std namespace.

  5) where would you define the enum? In a separate header file? In the same header, outside of the class (but in a namespace)?

The "namespace" part isn't necessary if you use a scoped enum. The place of the enum depends on its use. If it's only used together with this class, it's reasonable to keep it in the same header. If CarName can get used without CarFactory, it's probably a better idea to put it into another unit. But that depends on your use case.

  6) How should the unit test look like? I wrote the following test which actually fails on the indicated line.

And it should fail. You moved the ownership of whatever d_carMap[sa] pointed to outside. However, you didn't remove the actual std::unique_ptr from your map. It's still there, being the same as a nullptr. Therefore, your d_carMap.find() yields an iterator:

// creates a ClownCar, stores it in d_carMap
// in a pair (ClownCar, <some unique_ptr>) and
// immediately moves from the <unique ptr>
// thus d_carMap contains (ClownCar, nullptr)
// after the call.
auto myCar = factory.getCar(ClownCar);

// find(ClownCar) finds (ClownCar, nullptr)
auto myNullCar = factory.getCar(ClownCar);

// whoops

So the problem is that you don't get rid of the nullptr std::unique_ptr in your map. And your test have shown that, so they're actually helping.

Overall I think that you've abstracted the code a little bit too much.  It's intend isn't clear, since the documentation is missing. Also, the indentation isn't uniform, but that's a minor problem.


Decimal-to-binary converter for IP addresses def dec_to_bin(ip):

    ip_array = ip.split(".")
    ip_array = filter(None, ip_array)

    if len(ip_array) != 4:
        return "Invalid IP Address format"

    else:
        ip_bin = []

        for x in range(len(ip_array)):

            # Formatting example referenced from:
            # http://stackoverflow.com/a/10411108/1170681

            ip_bin.append('{0:08b}'.format(int(ip_array[x])))
            ip_bin.append(".")

        ip_bin.pop()
        return ''.join(ip_bin)

This is a simple parser that will take a IP address in decimal form and convert it into its binary representation.

I'm looking for any tips on coding styles and improving the efficiency of the code.

Code simplification and related:

Line 4: remove line 3 and replace line 4 with

ip_array = filter(None, ip.split('.'))

Line 7:
Create an exception, to prevent that a caller gets a string of error instead of the actual result.

Line 12: assuming this is not python3, it's better if you use xrange() instead of range(), because range would create an actual list listing all the items in specified range. xrange(), instead, returns the next item in the sequence without creating the list. This behaviour is the same as the range() function in python3.
Anyway, for your purposes changing it with:

for el in ip_array:
    ip_bin.append('{0:08b}'.format(int(el)))

or, even better:

ip_bin = ['{0:08b}'.format(int(el)) for el in ip_array]

Last line:
instead of appending the "." after each element, join over the dot:

return '.'.join(ip_bin)

Style:
Pursue consistency in all the code: if you use double-quotes for defining strings, use them everywhere, same thing with single-quotes (which i personally prefer for strings, reserving double ones for docstrings).

Reassuming:

def dec_to_bin(ip):

    ip_array = filter(None, ip.split('.'))

    if len(ip_array) != 4:
        raise NotValidIPException('Invalid IP Address format.')
    else:
        ip_bin = ['{0:08b}'.format(int(el)) for el in ip_array]
        return '.'.join(ip_bin)


Using /proc/stat to calculate CPU usage Inspired by this question: To calculate the total CPU usage as a percentage

I wrote the following bash script (as an answer) to calculate the CPU usage for a (configurable) period over all CPU cores on a host.

#!/bin/bash

#CORECOUNT=$(grep cpu /proc/stat | grep -v 'cpu ' | wc -l)
#echo $CORECOUNT cores not needed for calculation though

DELAY=${1:-1}

function getstat() {
    grep 'cpu ' /proc/stat | sed -e 's/  */x/g' -e 's/^cpux//'
}

function extract() {
    echo $1 | cut -d 'x' -f $2
}

function change() {
    local e=$(extract $ENDSTAT $1)
    local b=$(extract $STARTSTAT $1)
    local diff=$(( $e - $b ))
    echo $diff
}

#Record the start statistics

STARTSTAT=$(getstat)

sleep $DELAY

#Record the end statistics

ENDSTAT=$(getstat)

#http://www.mjmwired.net/kernel/Documentation/filesystems/proc.txt#1236
#echo "From $STARTSTAT"
#echo "TO   $ENDSTAT"
#     usr    nice   sys     idle       iowait irq    guest
#From 177834 168085 1276260 3584351494 144468 154895 0 0 0 0
#TO   177834 168085 1276261 3584351895 144468 154895 0 0 0 0

USR=$(change 1)
SYS=$(change 3)
IDLE=$(change 4)
IOW=$(change 5)

#echo USR $USR SYS $SYS IDLE $IDLE IOW $IOW

ACTIVE=$(( $USR + $SYS + $IOW ))
TOTAL=$(($ACTIVE + $IDLE))
PCT=$(( $ACTIVE * 100 / $TOTAL ))

echo "BUSY $ACTIVE TOTAL $TOTAL $PCT %"

The key features I wanted to ensure are just a single read of the stat file at the beginning, and another single read at the end.

There is some debate about what constitutes 'active' time (Is IOWait busy, or idle time?) If you have insights on that, I would appreciate that too.

All suggestions and criticisms welcome.

Nice script :-)

getstat is a bit odd:

It runs a grep and then a sed, when probably a single sed (or awk) could do the job
It replaces spaces with x, but not very clear why:

I see that extract uses x as the separator, but it could just as well use space for that
I also see that using x as the separator makes the change function slightly simpler, because it lets you write extract $ENDSTAT instead of extract "$ENDSTAT", but then again, having to quote doesn't seem a big problem

In extract, you run an echo and then a cut:

echo $1 | cut -d 'x' -f $2

In bash you can use here strings, which is better than using echo:

cut -d 'x' -f $2 <<< $1

But actually, if we keep the spaces as the separator in getstat, then we can rewrite both of these functions with fewer extra processes:

function getstat() {
    sed -ne '/^cpu  */s/^cpu  *//p' /proc/stat
    # that is:
    # -n to not print lines by default
    # for lines matching /^cpu  */, delete /^cpu  */, and print the line
}

function extract() {
    # example 1st param: "437797 1219 185341 549955584 58662 4 7083 0 0 0"
    # example 2nd param: 1, 2, 3, ... (the n-th column to use)
    n=$2          # saving the position param
    set -- $1     # reset positional params from $1, so that $1=437797, $2=1219, ...
    echo ${!n}    # echo the n-th positional param
}

function change() {
    local e=$(extract "$ENDSTAT" $1)
    local b=$(extract "$STARTSTAT" $1)
    echo $(( $e - $b ))
}


Lucas sequence implementation Could you suggest any improvements in this Lucas sequence's implementation:

from mpmath.libmp import bitcount as _bitlength

def _int_tuple(*i):
    return tuple(int(_) for _ in i)

def _lucas_sequence(n, P, Q, k):
    """Return the modular Lucas sequence (U_k, V_k, Q_k).
    Given a Lucas sequence defined by P, Q, returns the kth values for
    U and V, along with Q^k, all modulo n.
    """
    D = P*P - 4*Q
    if n < 2:
        raise ValueError("n must be >= 2")
    if k < 0:
        raise ValueError("k must be >= 0")
    if D == 0:
        raise ValueError("D must not be zero")

    if k == 0:
        return _int_tuple(0, 2, Q)
    U = 1
    V = P
    Qk = Q
    b = _bitlength(k)
    if Q == 1:
        # For strong tests
        while b > 1:
            U = (U*V) % n
            V = (V*V - 2) % n
            b -= 1
            if (k >> (b - 1)) & 1:
                t = U*D
                U = U*P + V
                if U & 1:
                    U += n
                U >>= 1
                V = V*P + t
                if V & 1:
                    V += n
                V >>= 1
    elif P == 1 and Q == -1:
        # For Selfridge parameters
        while b > 1:
            U = (U*V) % n
            if Qk == 1:
                V = (V*V - 2) % n
            else:
                V = (V*V + 2) % n
                Qk = 1
            b -= 1
            if (k >> (b-1)) & 1:
                t = U*D
                U = U + V
                if U & 1:
                    U += n
                U >>= 1
                V = V + t
                if V & 1:
                    V += n
                V >>= 1
                Qk = -1
    else:
        # The general case with any P and Q
        while b > 1:
            U = (U*V) % n
            V = (V*V - 2*Qk) % n
            Qk *= Qk
            b -= 1
            if (k >> (b - 1)) & 1:
                t = U*D
                U = U*P + V
                if U & 1:
                    U += n
                U >>= 1
                V = V*P + t
                if V & 1:
                    V += n
                V >>= 1
                Qk *= Q
            Qk %= n
    U %= n
    V %= n
    return _int_tuple(U, V, Qk)

Your _int_tuple function does not seem to serve any real purpose. U and V are always integers (you initialize them to 1 and do only arithmetic on them afterwards). And what exactly your program will do with a Q which is e.g. a string is hard to follow, but I think it will run into the else block (Q is neither 1 nor -1) and then try to do Qk = Q, followed by Qk *= Qk which fails for almost everything but number types.

I also can't see any parts of your code which would produce a float (though I might have overlooked one).

So I would get rid of that function and return just a tuple and see what it does.


Critique sanitized user email PHP Script So, I have a basic 'contact us' form that i have built, and I do some jQuery checking first (that a phone number is only numbers, email address doesn't contain unneeded characters, etc), but I also wanted to do some sanitization before sending out the email.

After reading many questions on SO (such a great service!) then doing some more reading on PHP.net and seeing some 'in use' code on W3Schools this is what I came up with. Is it secure? Is there more I should be doing? Is this overkill?

It should be said that the user content lives on my server for a very short amount of time and then is sent off to email services (GMail and HostGator)

<?
$name = filter_var($_POST[name], FILTER_SANITIZE_STRING);
$email = filter_var($_POST[email], FILTER_SANITIZE_EMAIL);
$phone = filter_var($_POST[phone], FILTER_SANITIZE_NUMBER_INT);
$usermessage = filter_var($_POST[message], FILTER_SANITIZE_STRING);
$businessmessage = filter_var($_POST[businessmessage], FILTER_SANITIZE_STRING);

$to = "spokedcouriers@gmail.com, tom@spokedcouriers.com";
$subject = "Incoming from SpokedCouriers.";
$message =
"Submitted From: ".$name."
".
"Email Address: ".$email . "
" .
"Phone Number: ".$phone . "
" .
"Message: ".$usermessage . "
" .
"Possible Business Message From: ".$businessmessage."
";

$headers  = 'MIME-Version: 1.0' . "\r\n"
    .'Content-type: text/html; charset=iso-8859-1' . "\r\n"
    .'From: ' . $email . "\r\n"
    .'Reply-To: ' . $email . "\r\n";

//GO AWAY.
mail($to, $subject, $message, $headers);
?>

Ok, this may come across as blunt or harsh, but IMO, that's what CodeReview has to do, as I explained earlier here.
So please, keep that in mind while reading this post.

Before I even start dealing with your code, I'd like to point out to you that you're using an unreliable, incomplete and misleading resource: w3schools. Sure, it may get some things right, but IMO, it's a misleading site. Many think the site is condoned, or even part of the W3C org. This is not the case, Please check the criticisms.
You are, of course, free to use w3schools, but be aware that it's not the best resource out there.

Is it secure?
No. I'd like to be kind about this, but that wouldn't help you. No, your code is still vulnerable to things like mail injection. I could easily copy paste some mail headers into your form and have your server, seemingly, forward a message sent yesterday by your rich, recently so tragically deceased alienated uncle in Barbados who left you his entire fortune.
I've included a link on mail injection in the bottom of this answer. This is an important issue that you need to address, still.

Now, the code:

I'll be going over your code block by block, sometimes even line-by-line:

<?

Don't use the short tag! Never. Period. It's not a lot of effort to write <?php instead, and it's far more reliable (<?php won't be confused with the odd <? xml...), and doesn't rely on the short-tags being enabled on your server.

$name = filter_var($_POST[name], FILTER_SANITIZE_STRING);
$email = filter_var($_POST[email], FILTER_SANITIZE_EMAIL);
$phone = filter_var($_POST[phone], FILTER_SANITIZE_NUMBER_INT);
$usermessage = filter_var($_POST[message], FILTER_SANITIZE_STRING);
$businessmessage = filter_var($_POST[businessmessage], FILTER_SANITIZE_STRING);

Ok, this block is essentially doing the same thing several times, and suffers from the same issue throughout.
First off, when setting the error reporting level to the Correct debugging levels of E_STRICT | E_ALL, you'll see a ton of notices and warnings pop up:

PHP Notice: Use of undefined constant name - assumed 'name'

This means you forgot to quote the array keys, PHP will helpfully assume you meant 'name', but it will issue a notice. This makes your code look sloppy and amateurish. More over, it costs resources, slowing your code down.

PHP Notice:  Undefined index: name

If your script is being executed and none, or not all of the post parameters have been sent, you're still accessing those keys in the $_POST super global. At no point are you checking if there has been a POST request.
This can produce the notice above, for each time you access the POST array. Add some basic checks:

<?php//full tag
if (!$_POST) exit();//no post request, no magic!
$name = isset($_POST['name']) ? filter_var($_POST['name'], FILTER_SANITIZE_STRING) : null;

Don't "over-filter"!

Where your usage of filter_var($email, FILTER_VALIDATE_EMAIL) is to be commended, your code:

$phone = filter_var($_POST[phone], FILTER_SANITIZE_NUMBER_INT);

is not the best of ideas. Now I don't know how you get the input from the user, but if I were to punch in "+10800123456", I'd translate that + to double 0, to standardize the input.
Apart from that FILTER_SANITIZE_NUMBER_INT will clean up input like "123.313.456,1", removing the comma and dots, but it will leave the dashes and plusses in "+123-313+456-1" untouched. If I were you, I'd simply do something like:

if ($phone{0} == '+')
    $phone = '00'.$phone;
preg_replace('/[^0-9]+/','',$phone);

This expands a leading + to double zeroes and removes everything Except digits from the string. You can then easily check the length, to determine (roughly) the validity of the input. If it's 3 digits long, it probably isn't valid.

mail is good for testing, less so for production

Sure mail is easy (once you get your server settings worked out), and does what it says on the tin: it sends an email.
However, sometimes you might require an alt-body (text and HTML), sometimes you may want to add attachments. Writing those headers all the time is a bit of a faff, and has been done before. Good programmers are lazy: they won't bother writing code that exists, and is free to use. The PHPMailer class is in common usage and is very easy to set up, and quite versatile. Though, as you can see here, still somewhat open to mail injection.

In terms of security, PEAR::Mail is still the better option (though not in terms of clean code IMO). Read through the link, and learn about mail injection some more.


N-puzzle program solved using A* search I coded the following program to solve the n-puzzle problem with n = 3. Are there any suggestions regarding readability and variable names?

import heapq
from random import shuffle
import math

n=3
fBoard = [1,2,3,
          4,5,6,
          7,8,0]

board = [i for i in range(9)]
shuffle(board)

print(board)
aStar()

class Board():

    def __init__(self,boardList,cost,parent):
        self._array = boardList
        self.heuristic = calcHeuristic(self._array)
        self.cost = cost
        self.totalCost = self.cost + self.heuristic
        self.parent = parent
        self.hashvalue = hash(tuple(self._array))

    def _printBoard(self):
        for var in range(len(self._array)):
            if var%3==0 and var!=0:
                print "\n",self._array[var],",",
            else:
                print self._array[var],",",

    def __hash__(self):
        return self.hashvalue

    def __eq__(self,other):
        return self._array == other._array

def aStar():
    pq = []
    cost = {}
    visited = {}
    start = Board(board,0,None)
    end = Board(fBoard,99,None)
    heapq.heappush(pq,(start.totalCost,start))
    while pq:
        tmp_tuple = heapq.heappop(pq)
        tmp_board = tmp_tuple[1]
        if tmp_board.heuristic == 0:
            end = tmp_board
            break

        index = tmp_board._array.index(0)
        x = index/3
        y = index%3
        listOfMoves = checkMove(x,y)

        for move in listOfMoves:
            moveBoard = tmp_board._array[:]
            moveIndex = move[0]*3 + move[1]
            moveBoard[index],moveBoard[moveIndex] = moveBoard[moveIndex],moveBoard[index]
            newBoard = Board(moveBoard,tmp_board.cost+1,tmp_board)
            new_cost = newBoard.totalCost
            if newBoard not in visited or new_cost < cost[newBoard]:
                cost[newBoard] = new_cost
                visited[newBoard] = 1
                newBoard.parent = tmp_board
                heapq.heappush(pq,(newBoard.totalCost,newBoard))

    var = end
    while var != start:
        print "\n"
        var._printBoard()
        var = var.parent

    print "\n"
    var._printBoard()

def manhattanDist(index,element):
    idx = fBoard.index(element)
    manhattan = 0
    fBoard_x = idx/3
    fBoard_y = idx%3
    x = index/3
    y = index%3
    manhattan += math.fabs(x-fBoard_x)
    manhattan += math.fabs(y-fBoard_y)
    return manhattan

def calcHeuristic(array):
    boardList = array
    heuristic = 0
    for var in boardList:
        x = var/3
        y = var%3
        if fBoard.index(var) != boardList.index(var):
            heuristic+=1
        heuristic+=manhattanDist(boardList.index(var),var)
    return heuristic

def checkMove(x,y):
    listOfMoves = [[x,y]]
    if(x+1<n):
        listOfMoves.append([x+1,y])
    if(x-1>=0):
        listOfMoves.append([x-1,y])
    if(y-1>=0):
        listOfMoves.append([x,y-1])
    if(y+1<n):
        listOfMoves.append([x,y+1])

    return listOfMoves

Are there any suggestions regarding readability and variable names?

You should follow PEP8, the official Python coding style guide.
For example:

Put spaces around operators. Instead of var%3==0, write as var % 3 == 0
Put spaces after commas separating parameters. Instead of Board(fBoard,99,None), write as Board(fBoard, 99, None)
Use snake_case for naming variables and methods instead of camelCase
Use PascalCase for naming classes

What is an fBoard? It's not easy to guess, so that's not a good name.

Don't execute code in global scope

These lines are executed immediately when the script is imported,
which is not good:

n=3
fBoard = [1,2,3,
          4,5,6,
          7,8,0]

board = [i for i in range(9)]
shuffle(board)

print(board)
aStar()

It would be better to move them into a main function,
and call that function from a if __name__ == '__main__' guard, like this:

def main():
    n=3
    fBoard = [1,2,3,
              4,5,6,
              7,8,0]

    board = [i for i in range(9)]
    shuffle(board)

    print(board)
    aStar()

if __name__ == '__main__':
    main()

This will make your script more reusable, and it will help you avoid some obscure bugs that can happen by variable shadowing.

Get ready for Python 3

It's not too difficult to make this script work with Python 3 too.
All it takes is adapting your print statements a little bit.
In some cases it's as trivial as adding (...), for example:

print "\n"

Rewrite as:

print("\n")

When you don't want the automatic newline, for example here:

print "\n",self._array[var],",",

You can add the end='' parameter:

print("\n",self._array[var],",", end='')


Print largest two integers from a stream I'm writing a program for my intro to comp sci course where I'm supposed to prompt the user to enter a stream of integers, and then return the largest two. We were given the hint to use a scanner and a while loop with the condition .hasnext.

Please review my code:

package Homework;

import java.util.*;

public class Question1 {

public static void main(String[] args) {
    System.out.println("Please type your integers");
    Scanner kbd = new Scanner(System.in);
    int big = 0;
    int big2 = 0;
    while(kbd.hasNext())
    {
       int num = kbd.nextInt();
       if(num > big) { // opens If statement
          big2 = big;
          big = num;
        } else if((num < big) && (num > big2))  {
           big2 = num;
        }
    }
    System.out.print(big);
    System.out.print(big2);
 }
}

Your logic is actually correct.  Perhaps you just don't know what to type in your terminal window to terminate the input stream.  After typing in your numbers, you need to send an end-of-file ("EOF") character.  On Unix, you do that by hitting ControlD.  On Windows, it's ControlZ.

Note that System.in is the program's "standard input" stream, which doesn't have to come straight from the keyboard.  You can also do

echo 3 1 4 1 5 9 | java Homework.Question1

or

java Homework.Question1 < numbers.txt

Therefore, kbd is a misnomer.  Try naming your Scanner more generically, like scanner or input.

You don't want to print your two numbers with no space in between.  It will look like one long number!

The placement of your braces is inconsistent, with the ones for the main function and the else if being closest to conventions.  The brace opening the while loop should be on the same line as the while keyword.  The brace closing the if block should be pulled further left, aligned with the if keyword, and else can come immediately after.

public class Question1 {
    public static void main(String[] args) {
        ...
        while (...) {
            ...
            if (...) {
                ...
            } else if (...) {
                ...
            }
        }
        System.out.println(...);
        System.out.println(...);
    }
}

You have interpreted the exercise such that duplicate entries in the input are ignored.  That's the easier way.  If the exercise requires you to print the same number twice when there is a tie for the maximum, then the program is trickier to implement.


Compare and match results in two two-dimensional arrays in Excel VBA I have four loops to populate and compare two two-dimensional arrays then add the results to the first array before writing back to the active worksheet.
Wondering if anyone has a cleaner alternate or more elegant method?
It works as is but haven't tested it on large arrays yet!

Function lastRow(x As Range, y As Worksheet)

lastRow = y.Cells(Rows.Count, x.Column).End(xlUp).Row

End Function

Sub arrayMatch()

Dim arr1() As Variant
Dim arr2() As Variant
Dim rowX As Byte, colX As Byte, aX As Byte

Dim arr1x As Long, arr2y As Long

arr1x = lastRow(Range("A1"), ActiveSheet) 'returns the last row in the column for cell A1
arr2y = lastRow(Range("A1"), Sheets(2)) 'returns the last row in the column for cell A1 on sheet2

ReDim arr1(1 To arr1x, 1 To 4) As Variant 'dynamically sizes arr1 array
ReDim arr2(1 To arr2y, 1 To 2) As Variant 'dynamically sizes arr2 array

For rowX = 1 To arr1x 'arr1x is the last row in the active sheet and the end of array arr1
    For colX = 1 To 3 'only fills array up to 3rd dimension as 4th is reserved for the match results

        arr1(rowX, colX) = Cells(rowX, colX).Value 'set the Cells range to whatever your array is

    Next colX
Next rowX

For rowX = 1 To arr2y 'arr2y is the last row in sheet 2 and the end of array arr2
    For colX = 1 To 2

        arr2(rowX, colX) = Sheets(2).Cells(rowX, colX).Value

    Next colX
Next rowX

For aX = 1 To arr1x
    For rowX = 1 To arr2y
        If arr1(aX, 3) = arr2(rowX, 1) Then
            arr1(aX, 4) = arr2(rowX, 2)
            rowX = arr2y 'helps to exit array earlier when a match is found
        End If
    Next rowX
Next aX

For rowX = 1 To arr1x
    Cells(rowX, 4).Value = arr1(rowX, 4)
Next rowX

End Sub

Couple notes -

Your comments aren't needed. Comments - "code tell you how, comments tell you why". The code should speak for itself, if it needs a comment, it might need to be made more clear. If not, the comment should describe why you're doing something rather than how you're doing it. Here are a few reasons to avoid comments all together.
It's good practice to indent all of your code that way Labels will stick out as obvious.
Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know.

Variable names - give your variables meaningful names:

arr1 - this is the first array for comparison, why not describe what it is? Same for arr2.
What are the rowX, colX, ax, arr1x, arr2y? The capitalization isn't consistent - Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names.

Creating and populating your arrays. I don't think you need the lastRow function. If you do, I'd use Private Function LastRow(ByVal targetRange as Range) as Long and just ensure you explicitly pass the range with the sheet qualifier.

If so, you can eliminate some variables as well

Dim arr1() As Variant
Dim arr2() As Variant
Dim lastRow As Long
lastRow = x 'target for firstarray
ReDim arr1(1 To lastRow, 1 To 4) As Variant 'dynamically sizes arr1 array
lastRow = x 'target for firstarray
ReDim arr2(1 To lastRow, 1 To 2) As Variant 'dynamically sizes arr2 array

As far as reusing the lastRow variables, just use Ubound(targetArray) instead.

Dim i As Long

For i = 1 To UBound(firstArray)
    firstArray(i, 1) = Cells(i, 1)
    firstArray(i, 2) = Cells(i, 2)
    firstArray(i, 3) = Cells(i, 3)
    secondarray(i, 1) = Sheet2.Cells(i, 1)
    secondarray(i, 2) = Sheet2.Cells(1, 2)
Next i

I'm not sure what you're doing here - populating the array? The other answer covers this - just populate it ater you dim it undimensioned.

For aX = 1 To arr1x
    For rowX = 1 To arr2y
        If arr1(aX, 3) = arr2(rowX, 1) Then
            arr1(aX, 4) = arr2(rowX, 2)
            rowX = arr2y 'helps to exit array earlier when a match is found
        End If
    Next rowX
Next aX

resetting rowX = arr2y is not the proper way to exit the loop:

If arr1(aX, 3) = arr2(rowX, 1) Then
   arr1(aX, 4) = arr2(rowX, 2)
   Exit For
End If

Also, why are you using aX as a byte for this? I don't understand that.

You can simplify the loops populating the arrays, combine everything into 1 loop and avoid looping 4 times. That should speed it up a bit. And I would really urge you to get rid of the arr1x and arr2y in favor or Ubound.


Search for data and order by priority efficiently I have a search-function, that searches for specific data within a huge Object.

Depending on how good a found entry matches, the finding gets "matchPoints" (or "Priority")

var findings = {};
for(var element in hugeObject){
    var points = getMatchPoints( inputToSearchFor, hugeObject[element] );
    if( points !== 0 ){    //It matched. At least a bit.
        findings[ points ] = element;
    }
}

Now I have all Elements, that somehow matched, within findings.

The function which implements this has to return an Array of all these elements. The most relevant result on the first place.

I currently use this for sorting:

var priorities = Object.keys(findings);    //Get the matchpoints of all findings
priorities.reverse(); //Sort with reverse order (highest priority / most matchpoints first)

var result = [];
for(var i=0; i<priorities.length; i++){
    result.push( findings[i] );
}
return result;

I have several issues with this code:

It is inefficient and probably very slow
The .reverse() function sorts the priorities alphabetically, not numerically
If 2 findings have the same amount of matchpoints, the findings-entry gets overwritten. So the result isn't even valid.

How can I solve this problem in an efficient and elegant way?

Edit: I should mention, that although the object where I search is very big, I only get few valid results which matchPoints > 0. In fact, most of the time I'll get only 2-3 matches at all.

The .reverse() function sorts the priorities alphabetically, not numerically

This is because Object.keys returns an array of strings.

  If 2 findings have the same amount of matchpoints, the findings-entry gets overwritten. So the result isn't even valid.

Right, you want to push matches onto the array and sort it afterwards.

var findings = [];
for (var element in hugeObject) {
    var points = getMatchPoints(inputToSearchFor, hugeObject[element]);
    if (points) {
        // It matched. At least a bit.
        findings.push({ points: points, element: element });
    }
}
findings.sort(function(a, b) { return b.points - a.points; });

Here I'm creating a new object with the number of match points stored in points and the found object stored in element, but you could store the point count directly in the found object if it makes more sense.


Parsing a BigRational efficiently I'm using the BigRational beta off the BCL CodePlex (bcl.codeplex.com), and I realized it had no parsing method, so I tried to write one. However, it's quite inefficient (5.5ms for a 254 character number). That's over 45x slower than the BigInteger implementation. I'd like to lower it to .5ms if that's even possible.

private static Regex DigitDotDigit = new Regex(@"^(\-|\+)?(\d+\.\d+)$", RegexOptions.Compiled);
private static Regex PlainDigit = new Regex(@"^(\-|\+)?\d+$", RegexOptions.Compiled);
private static Regex DigitSlashDigit = new Regex(@"^(\-|\+)?\d+/\d+$", RegexOptions.Compiled);
private static Regex DotDigit = new Regex(@"^(\-|\+)?(\.\d+)", RegexOptions.Compiled);
private static bool RegexInitiated = false;
public static bool TryParse(string parse, out BigRational result)
{
    if (DigitDotDigit.IsMatch(parse))
    {
        int zeros;
        bool isNegative = false;
        string[] parts = parse.TrimStart('+').Split('.');
        parts[1] = parts[1].TrimEnd('0');
        if (parts[0].StartsWith("-"))
        {
            isNegative = true;
            parts[0] = parts[0].Substring(1);
        }
        BigRational whole = new BigRational(BigInteger.Parse(parts[0]), BigInteger.Zero, BigInteger.One);
        BigRational decimalPart = new BigRational(BigInteger.Parse(parts[1]), BigInteger.Zero, BigInteger.One);
        zeros = parts[1].Length - parts[1].TrimStart('0').Length;
        toSubtract = toSubtract + zeros;
        if (zeros > 0)
        {
            toSubtract = toSubtract - 1;
        }
        while (toSubtract != 0)
        {
            decimalPart /= 10;
            toSubtract = toSubtract - 1;
        }
        result = whole + decimalPart;
        if (isNegative)
        {
            result = -result;
        }
        return true;
    }
    else if (DotDigit.IsMatch(parse))
    {
        return TryParse("0" + parse, out result);
    }
    else if (PlainDigit.IsMatch(parse))
    {
        parse = parse.TrimStart('+');
        if (parse.StartsWith("-"))
        {
            result = new BigRational(-BigInteger.Parse(parse), BigInteger.Zero, BigInteger.One);
            return true;
        }
        result = new BigRational(BigInteger.Parse(parse), BigInteger.Zero, BigInteger.One);
        return true;
    }
    else if (DigitSlashDigit.IsMatch(parse))
    {
        string[] parts = parse.TrimStart('+').Split('/');
        if (parts[0].StartsWith("-"))
        {
            parts[0] = parts[0].Substring(1);
            result = -(new BigRational(BigInteger.Parse(parts[0]), BigInteger.Parse(parts[1])));
            return true;
        }
        result = new BigRational(BigInteger.Parse(parts[0]), BigInteger.Parse(parts[1]));
        return true;
    }
    result = BigInteger.Zero;
    return false;
}

The answer, I found out now, was that if I moved the Regexes to private fields, and then used those, after the first use it will radically drop down to .4ms, and that's including a WriteLine call. So the real hotspot in this method was the Regex. Thanks everyone!


Stack Exchange userscript for showing spoilers on questions with particular tags Stack Exchange sites support hiding text as a spoiler with >!; for example:

   This text will be hidden until you hover over it/tap the "show spoiler" button.

This gets a lot of use on the Sci-Fi & Fantasy stack, usually to cover up key plot elements of a particular work (like the recent Star Wars film).

That’s a good way to avoid spoilers if you haven’t seen the work in question, but it can be annoying if you already know the plot.  Over on Meta SFF, somebody asked whether it would be possible to unhide spoilers on questions with particular tags.

I wrote this UserScript, which allows the user to specify an array of tags, and will unhide spoilers on questions with those tags:

// ==UserScript==
// @name         SFF.SE Show spoilers on questions with particular tags
// @namespace    http://tampermonkey.net/
// @version      1.0
// @description  Show all spoilers on SFF.SE questions with specified tags
// @author       alexwlchan
// @match         *://scifi.stackexchange.com/questions/*
// @match         *://meta.scifi.stackexchange.com/questions/*
// @grant        none
// ==/UserScript==
/* jshint -W097 */
'use strict';

// If every tag on a question is in this array, then the spoilers on this
// question will be shown.
var SHOW_ME_SPOILERS_IF_TAGGED = [
    "harry-p0tter",
    "star-w4rs",
];

// Returns an array of tags on a question
var getTagsOnQuestion = function() {
    var taglist = document.getElementsByClassName("post-taglist")[0];
    var hrefs = taglist.getElementsByClassName("post-tag");

    var tags = [];
    for (var ii = 0; ii < hrefs.length; ii++) {
        tags.push(hrefs[ii].innerText);
    }
    return tags;
}

// Returns true/false depending on whether the spoilers on this question
// should be shown -- that is, whether every tag on the question is in
// SHOW_ME_SPOILERS_IF_TAGGED.
var shouldShowSpoilersOnQuestion = function() {

    // Get the tags on a question.  All questions have at least one tag.
    var tags = getTagsOnQuestion();

    for (var jj = 0; jj < tags.length; jj++) {
        // If we don't find the tag in the array of spoiler-able tags,
        // then we have to reject the question.
        if (SHOW_ME_SPOILERS_IF_TAGGED.indexOf(tags[jj]) == -1) {
            return false;
        }
    }
    return true;
}

// Modify all the spoilers on the page to show their spoiler text
var showSpoilers = function() {
    var spoilers = document.getElementsByClassName("spoiler");
    for (var kk = 0; kk < spoilers.length; kk++) {
        spoilers[kk].transition = "none";
        spoilers[kk].style.color = "#222426";
    }
}

if (shouldShowSpoilersOnQuestion()) {
    showSpoilers();
}

Any feedback is welcome, but particularly:

Is this good/idiomatic JavaScript?
Is the intent of the code clear?
Are there any edge cases that I’ve missed?
Is there a better way to check if (tags on the question) is a subset of (tags the user wants spoilers to be shown for)? I feel like my current approach is quite inefficient.

For Loop

The style of the for loop you're using could be improved.

for (var jj = 0; jj < tags.length; jj++) {
    // If we don't find the tag in the array of spoiler-able tags,
    // then we have to reject the question.
    if (SHOW_ME_SPOILERS_IF_TAGGED.indexOf(tags[jj]) == -1) {
        return false;
    }
}

should probably be changed to a for of loop

for (tag of tags) {
    // If we don't find the tag in the array of spoiler-able tags,
    // then we have to reject the question.
    if (SHOW_ME_SPOILERS_IF_TAGGED.indexOf(tag) == -1) {
        return false;
    }
}

map

This code is essentially doing the same thing as a map function:

var tags = [];
for (var ii = 0; ii < hrefs.length; ii++) {
    tags.push(hrefs[ii].innerText);
}

You could rewrite it to this (which uses an arrow function as well):

var tags = hrefs.map(href => href.innerText);


Check if clicked element has specific tagname I'm trying to determine whether an element which has been clicked has a specific tagname. The trick is, that sometimes when you click an element you want any parent to be that specific tag.

My approach:

var ignore = 'a input textarea body'.split(' ');

var isClickable = function(elem) {
    var cancel = false;

    for (i in ignore) {
        if (elem.nodeName.toLowerCase() !== ignore[i]) {
            if (elem.parentNode.nodeName.toLowerCase() !== 'body')
                isClickable(elem.parentNode);
        }
        else {
            cancel = true;
            return cancel;
        }
    }

    return cancel;

};

document.onclick = function(evt) {
    isClickable(evt.target);
}

Is that code right or did I forget something?

SOLUTION: I came up to that tiny script based on Guffas solution (thanks!)
I've optimized the code to a shorter and faster version:

var isClickable = function(elem) {
    for (i in ignore)
        if (elem.nodeName.toLowerCase() === ignore[i])
            return true;
    return (elem.parentNode.nodeName.toLowerCase() !== 'body') && isClickable(elem.parentNode);
};

I see two big problems with the code:

You are doing the recursive call inside the loop, so you will do each level four times. That will quickly escalade to a lot of calls. You should do the call after the loop, where you have determined that the node is not one in the array.
You are ignoring the result of the recursive call.

So:

function isClickable(elem) {
  for (i in ignore) {
    if (elem.nodeName.toLowerCase() === ignore[i]) {
      return true;
    }
  }
  if (elem.parentNode.nodeName.toLowerCase() !== 'body') {
    return isClickable(elem.parentNode);
  }
  return false;
};


Reverse a sentence in Java You are given an array of characters arr that consists of sequences of characters separated by space characters. Each space-delimited sequence of characters defines a word.

Implement a function reverseWords that reverses the order of the words in the array in the most efficient manner.

Explain your solution and analyze its time and space complexities.

Example:

input:  arr = [ 'p', 'e', 'r', 'f', 'e', 'c', 't', '  ',
                'm', 'a', 'k', 'e', 's', '  ',
                'p', 'r', 'a', 'c', 't', 'i', 'c', 'e' ]

output: [ 'p', 'r', 'a', 'c', 't', 'i', 'c', 'e', '  ',
          'm', 'a', 'k', 'e', 's', '  ',
          'p', 'e', 'r', 'f', 'e', 'c', 't' ]

Constraints:

[time limit] 5000ms

[input] array.character arr

0 ≤ arr.length ≤ 100
[output] array.character

My approach:

import java.io.*;

import java.util.*;

class Solution {

  static void reverseAword(char [] arr, int start, int end)
  {
    int len = end - start + 1;
    for( int i = start, j = end; i < len/2; i++,j-- )
    {
        char temp = arr[i];
        arr[i] = arr[j];
        arr[j] = temp;
    }
  }

  static char[] reverseWords(char[] arr) {
    // your code goes here
    int len = arr.length;

    //Reverse the complete sentence
    reverseAword(arr,0,len - 1);

    int start = 0;
    for( int i = 0; i < len; i++)
    {
      //If there is a space, reverse the word from start till index - 1( before ' ')
      if( arr[i] == ' ' )
      {
        if( start == 0 )
          {
            reverseAword(arr,start,i - 1);
            start = 0;
          }
      }
        else if( i == len - 1)
        {
          if( start != 0)
              reverseAword(arr, start,i);
        }
        else
        {
          if( start == 0)
              start = i;
        }

    }

    return arr;
  }

  public static void main(String[] args) {

  char[] c1 = {' ', ' '};
    c1 = reverseWords(c1);

    System.out.println(Arrays.toString(c1));
  }

}

I have the following questions with respect to the above code:

1) How can I further improve the time and space complexity?

2) Can this problem be solved using a smarter way?

3) Are there too many redundant variables that we are better off with?

Reference

You can improve time complexity by doubling the space complexity. If you don't modify the input array in place (which is probably a bad idea in real code anyway), you can use System.arraycopy instead of repeatedly reversing parts of the char[]. I also think that would be easier to read. That approach might look something like:

import java.util.Arrays;

public final class Solution {

    private static final char[] INPUT =
            new char[] {
                    'p', 'e', 'r', 'f', 'e', 'c', 't', ' ',
                    'm', 'a', 'k', 'e', 's', ' ',
                    'p', 'r', 'a', 'c', 't', 'i', 'c', 'e' };

    private static final char[] OUTPUT =
            new char[] {
                    'p', 'r', 'a', 'c', 't', 'i', 'c', 'e', ' ',
                    'm', 'a', 'k', 'e', 's', ' ',
                    'p', 'e', 'r', 'f', 'e', 'c', 't' };

    public static void main(final String[] argv) {
        System.out.println(Arrays.equals(OUTPUT, reverseWords(INPUT)));
    }

    private static char[] reverseWords(final char[] input) {
        final char[] output = new char[input.length];
        int outputIndex = 0;
        int lastSpaceIndex = input.length;
        for (int i = input.length - 1; i >= 0; i--) {
            if (input[i] == ' ') {
                final int length = lastSpaceIndex - (i + 1);
                System.arraycopy(input, i + 1, output, outputIndex, length);
                output[outputIndex + length] = ' ';
                outputIndex += length + 1;
                lastSpaceIndex = i;
            }
        }
        System.arraycopy(input, 0, output, outputIndex, lastSpaceIndex);
        return output;
    }
}


jQuery plugin which will print to a printer an element or a jQueryUI Dialog I have a need to print the content of a jQueryUI Dialog to a printer, and I couldn't find anything to my liking.  I developed the following simple jQuery plugin which prints one or more provided HTML elements, and am using it to print the jQueryUI Dialog DIV element.  The code is below, and a live example is located here.

It works on FF 26.0.  However, after printing a couple of times, FF asks the user if popups should be disabled, which I wish not to happen.  Also, it doesn't work with older IE, and likely other browsers.  Don't worry because, when printing, you still need to click the operating system print dialog, so you won't waste any paper!

Please provide any recommendations on how this plugin could be improved.

Actual Plugin:

/*
* jQuery printIt
* Print's the selected elements to the printer
* Copyright Michael Reed, 2014
* Dual licensed under the MIT and GPL licenses.
*/
(function($){
    var defaults = {
        elems           :null, //Element to print HTML
        copy_css        :false,//Copy CSS from original element
        external_css    :null  //New external css file to apply
    };

    var methods = {
        init : function (options) {
            var settings = $.extend({}, defaults, options)
            elems=$(settings.elems);
            return this.each(function () {
                $(this).click(function(e) {
                    var iframe   = document.createElement('iframe');
                    document.body.appendChild(iframe);
                    $(iframe).load(function(){
                        elems.each(function(){
                            iframe.contentWindow.document.body.appendChild(this.cloneNode(true));
                        });
                        if(settings.copy_css) {
                            var arrStyleSheets = document.getElementsByTagName("link");
                            for (var i = 0; i < arrStyleSheets.length; i++){
                                iframe.contentWindow.document.head.appendChild(arrStyleSheets[i].cloneNode(true));
                            }
                            var arrStyle = document.getElementsByTagName("style");
                            for (var i = 0; i < arrStyle.length; i++){
                                iframe.contentWindow.document.head.appendChild(arrStyle[i].cloneNode(true));
                            }
                        }
                        if(settings.external_css) {
                            var style  = document.createElement("link")
                            style.rel  = 'stylesheet';
                            style.type = 'text/css';
                            style.href = settings.external_css;
                            iframe.contentWindow.document.head.appendChild(style);
                        }
                        var script   = document.createElement('script');
                        script.type  = 'text/javascript';
                        script.text  = 'window.print();';
                        iframe.contentWindow.document.head.appendChild(script);
                        $(iframe).hide();
                    });
                });
            });
        },
        destroy : function () {
            //Anything else I should do here?
            delete settings;
            return this.each(function () {});
        }
    };

    $.fn.printIt = function(method) {
        if (methods[method]) {
            return methods[method].apply(this, Array.prototype.slice.call(arguments, 1));
        } else if (typeof method === 'object' || ! method) {
            return methods.init.apply(this, arguments);
        } else {
            $.error('Method ' +  method + ' does not exist on jQuery.printIt');
        }
    };
    }(jQuery)
);

To configure:

$(function () {
    $("#openDialog").click(function () {
        $("#myDialog").dialog('open')
    });
    $("#myDialog").dialog({
        modal: true,
        autoOpen: false
    });
    $('#printIt').printIt({
        elems: $("#myDialog"),
        copy_css: true,
        external_css: 'test2.css'
    });
});

From a once over:

I am not a native English speaker but //Element to print HTML confused me. Maybe //HTML elements to be printed?
delete settings; is meaningless in destroy ( see comment )
elems=$(settings.elems); should be var elems = $(settings.elems); otherwise you pollute the global namespace
You declare var i twice in a for loop
You should consider removing the iframe in destroy

All in all very clean, maintainable code.