# Eight queens in C++

The idea behind this problem is to represent a configuration of 8 queens with a one-dimensional array. So a represents the first row, a the second row etc. and a[row]=column position, where column position represents the "row position" of the queen.

Is there anything I can improve?

#include <iostream>
#include <math.h>
using namespace std;

int main() {

int a;
int m, b, i=0, x, y, k=0, c,u;

for(u=0; u<8; u++){
cin>>a[u];
}

for(m=0; m<7; m++){

for(b=m+1; b<=7; b++){

if(a[m]==a[b]) i++;
}

}
for(x=0; x<7; x++){
c=0;
for(y=x+1; y<=7; y++){
c++;
if(abs(a[x]-a[y])==c) k++;

}
}

if(i==0 && k==0) cout<<"valid";
else cout<<"invalid";
}


## 1 Answer

• using namespace std is always wrong. In this case it is unjustified as well.

• int a only has room for 7 elements. You put 8 there. This is plain UB.

• Avoid magic numbers. Define a const int board_size = 8. board_size and board_size - 1 are understood much better than a random mix of 7 and 8.

• An idiomatic counting for loop terminating condition is expressed with <, not <=:

    for (....; b < board_size; ....)

• Avoid single letter variables. Variable name usually conveys some meaning. In this case, board coordinates are traditionally called rank and file.

• Declare variables as close the use as possible (e.g. for (int x = 0; ...)).

• Terminate early. Instead of i++ you may immediately say std::cout << "invalid"; return 0; (same with k), thus eliminating unneeded variables.