1

I am doing the 8 queens problem and trying to implement the method which will check if there is any collision between queens. When it comes to the logic of the problem, I think I understand it, but I am getting NullpointerException and I can't figure out why. Any help would be highly appreciated.

private static boolean isOK(int[][] matrix) {
        boolean isInCollision = false;
        Queen [] queens = new Queen[8];
        for (int i = 0; i < matrix.length; i++) {
            for (int j = 0; j < matrix.length; j++) {
                if(matrix[i][j] == 1){
                    queens[i] = new Queen(i, j);

                }
            }
        }


        for (int i = 0; i < queens.length; i++) {
            for (int j = i+1; j < queens.length; j++) {
                if(queens[i].getX()==queens[j].getX() && Math.abs(queens[i].getX()-queens[j].getX())==Math.abs(queens[i].getY()-queens[j].getY())){
                    isInCollision = true;
                }
            }
        }
        return isInCollision;
    }

I get error in this line of code:

if(queens[i].getX()==queens[j].getX() && Math.abs(queens[i].getX()-queens[j].getX())==Math.abs(queens[i].getY()-queens[j].getY()))
mrGreenBrown
  • 576
  • 1
  • 7
  • 20
  • One of the "dot"-operations are returning `null`. This is why you might want to assign parts of complex expressions to local variables. This will allow you to identify better where the exception happened. – Thorbjørn Ravn Andersen May 07 '16 at 11:54

2 Answers2

1

In the double for loop add a test that checks if both queens are set if not skip using continue like that:

 for (int i = 0; i < queens.length; i++) {
    for (int j = i+1; j < queens.length; j++) {
        if (queens[i] == null || queens[j] == null) {
            continue;
        }
        // rest of the method here
    }
}

You should also add a break once a collision has been detected by giving a label to the main loop and using this label with your break as next:

main: for (int i = 0; i < queens.length; i++) {
    for (int j = i+1; j < queens.length; j++) {
        // previous test here
        if (collision detected) {
            isInCollision = true;
            break main;
        }
    }
}

This will break the double for loop as soon as a collision has been detected. Another way to do the same this is simply doing return true; instead of affecting true to isInCollision then returning the value after the loops.

Nicolas Filotto
  • 43,537
  • 11
  • 94
  • 122
0

This sounds like you didn't place queens on all places in queens[]:

if(matrix[i][j] == 1){
     queens[i] = new Queen(i, j);
}

Therefore, if one spot in queens[] is not set here, it will be null and therefore queens[j].getX() or queens[j].getY() will produce NPE. So, check if this array is filled correctly before doing any operation with it (do it between fors).

Adnan Isajbegovic
  • 2,227
  • 17
  • 27