2
\$\begingroup\$

In my previous NQueens Code Review question, I received feedback saying that I should write a program that would find all solutions. This revised program should be more general than before.

import java.util.*;

public class RedoNQueens {
    public static void main(String[] args) {
        Scanner input = new Scanner(System.in);
        System.out.print("Enter number of queens: ");
        int nQueens = input.nextInt();
        //Create board and clear it
        int[] board = new int[nQueens];
        for (int i = 0; i < board.length; i++) {
            board[i] = -1;
        }

        //Place the first piece to get started
        int row = 0;
        board[row] = 0;
        row++;
        int column = 0;
        long solutions = 0;
        while (row >= 0) {
            if (isValid(row, column, board)) {
                board[row] = column;
                row++;
                column = 0;

                //Board completed?
                if (row >= board.length) {
                    solutions++;
                    System.out.println("Solution " + solutions);
                    printBoard(board);
                    row--;
                    column = board[row] + 1;
                }
            }
            else {
                //If current column is not valid
                column++;
                if (column >= board.length) {
                    board[row] = -1;
                    row--;
                    if (row >= 0)
                        column = board[row] + 1;
                }
            }
        }
        System.out.println("Number of solutions for " + nQueens + " queens is " + solutions);
    }

    public static boolean isValid(int row, int column, int[] board) {
        if (column >= board.length)
            return false;

        for (int i = 0; i < row; i++) {
            //Check if column appears in any other row
            if (board[i] == column)
                return false;

            //Check both diagnals
            if (Math.abs(row - i) == Math.abs(column - board[i])) 
                return false;
        }

        return true;
    }

    public static void printBoard(int[] board) {
        for (int i = 0; i < board.length; i++) {
            for (int j = 0; j < board.length; j++) {
                if (board[i] == j)
                    System.out.print("|Q");
                else
                    System.out.print("| ");
            }
            System.out.println("|");
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

I'm not sure what you're looking for specifically in trying to revise your program past this point. So, here's a handful of general things I would change:

  • Remove static modifier from your code. This can be done by creating an NQueens class, that maybe takes the N as a parameter to it's constructor, then can be run by calling a "run" method.
  • Use curly braces on all your conditional statements. You're less likely to break code down the road that way.
  • See all those comments you have? Instead of commenting on little sections of that, you could increase your code's readability by making those their own methods.

    Example: In the following code, maybe make a method that explains what you're doing here, such as resetPosition or something.

        //If current column is not valid
        column++;
        if (column >= board.length) {
            board[row] = -1;
            row--;
            if (row >= 0)
                column = board[row] + 1;
        }
    
  • I would suggest trying to better name your iterators as well. i and j are just fine, but if you're actually iterating through something that has a name, such as row, column, etc. go ahead and use that. Again, easier to read and follow the algorithm.

  • Similarly, row, column aren't totally obvious in how they're used. Would they be better represented as currentRow or currentColumn? Since you're using these as nodes to find new solutions from, it might makes sense to have a more descriptive name.
  • isValid; what exactly is valid? Is that a valid representation of the board, a valid solution, or only part of the solution?
  • Lastly, why isn't the board represented as a 2D array? You received this as feedback last time as well, but I second it. A game board is represented in 2-dimensional space, x and y. The same way a cube would be in 3-dimensional space and therefor hopefully be represented as a 3-dimensional array. Trying to represent objects outside their native dimension takes some time to understand. I would avoid this at all costs unless there are serious performance gains to be made.

I think the point I'm getting at here is that: Ideally, I shouldn't have to know anything about what the NQueens problem is in order to follow your code without much issue.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.