UVA 750: 8 Queens Chess

In chess it is possible to place eight queens on the board so that no
one queen can be taken by any other. Write a program that will
determine all such possible arrangements for eight queens given the
initial position of one of the queens.

Input


The first line of the input contains the number of datasets, and it's followed by a blank line. Each dataset contains a
pair of positive integers separated by a single space that describes
the initial position of one of the 8 queens

Output


Output for each dataset will consist of a one-line-per-solution
representation.


Each solution will be sequentially numbered 1...N. Each solution will
consist of 8 numbers. Each of the 8 numbers will be the ROW coordinate
for that solution. The column coordinate will be indicated by the
order in which the 8 numbers are printed. That is, the rst number
represents the ROW in which the queen is positioned in column 1; the
second number represents the ROW in which the queen is positioned in
column 2, and so on

Sample Input

1

1 1

Sample Output

SOLN       COLUMN  
 #     1 2 3 4 5 6 7 8  

 1     1 5 8 6 3 7 2 4  
 2     1 6 8 3 7 4 2 5  
 3     1 7 4 6 8 2 5 3  
 4     1 7 5 8 2 4 6 3

The full description of the problem can be found here.

#include 
#include 
#include 
#include 

class Chess
{
    const int row_size, col_size;
    const int row_queen, col_queen;
    std::vector > sol;
    // queens_row_num[i] == on which row the queen is placed on col i
    int queens_row_num[8 + 1], idx_queens_row_num;

    bool queen_is_in_attack_pos(const int &row, const int &col)
    {
        for ( int j = 1; j  v;
        for ( int j = 1; j > T;
    for ( int t = 1; t > r >> c;
        Chess chess = Chess(r, c);

        chess.find_solutions();
        chess.print_solutions();
        if ( t < T )
            std::cout << std::endl;
    }
    return 0;
}

What are the possible way to improve:

• on quality-wise

• t

First, your indentation is a little crazy, which makes it very difficult to see which code is in which scope. Consider:

if ( queens_row_num[j] )
{
    if ( std::abs(queens_row_num[j] - row) ==
     std::abs(j - col) ||
     queens_row_num[j] == row || j == col )
    return true;
}

compared to:

if (queens_row_num[j])
{
    if (std::abs(queens_row_num[j] - row) == std::abs(j - col) ||
        queens_row_num[j] == row ||
        j == col)
    {
        return true;
    }
}

• I made each scope get its own indentation level.

• I gave each scope its own braces, which helps prevent bugs--especially important when working with improperly indented code.

• I put each part of the condition in the second if on its own line, with the boolean || operator at the end of the line. I always structure my ifs like this when the lines get too long. I split at the |, ||, &, &&, and ^ operators, but not the == operator or arithmetic/concatenation operators.

• This is purely stylistic, but I removed the spaces from the inside of the braces. You are pretty consistent about this, but have a few deviations. Consistency is more important here than the specific style you choose.

Notice how my version is easier to see what code is in what scope and executes when because you don't have to figure out what closing parenthesis and brace goes with which opening parenthesis and brace.

Overall, your naming is pretty good. print_solutions is an excellent name, but T, t, r, and c in main aren't so good. I assume the r and c stand for row and column? Why not name them that directly?

No using namespace std? Congratulations for not falling into that trap.

Using std::endl will flush the IO buffer, which ensures every character is printed before it lets you continue, and is a very expensive operation. You should just use \n (or System.Environment.NewLine if you are using Visual C++), and only flush the buffer at the end of the method, if necessary.

I have mixed feelings about this ctor:

Chess(const int &r, const int &c) : row_queen(r), col_queen(c),
                                    row_size(8), col_size(8)
{}

I like that you are setting the row size and column size in the ctor, which has a flavor of Dependency Injection and an extensible design; you can change those values to 10 and 10, and solve the more generic N-Queens problem very simply. The problem is that you hard-code the values in without giving the caller a chance to say which version they want solved; you should make these parameters for the caller to specify. Because the 8-Queens problem is the most common variant of the N-Queens, these parameters could be optional, which would let the caller solve the 8-Queens problem by creating:

Chess chess = Chess(setRow, setColumn);

and the N-Queens problem by calling:

Chess chess = Chess(setRow, setColumn, rows, columns);

StackExchange Code Review Q#150816, answer score: 6