0
\$\begingroup\$

So I have been working on this Chess project and for that I need a move generator. I wish to know if I can improve my code for the move generation for the Knight. Here is my current code;

Let me tell a few things so you can better understand it. I am using vector<vector<int>> for the main pseudo moves. I use add the move to a vector<int> first, then I add it to the pseudomoves.

The board is simply represented with int board[8][8]. Here are the values and the pieces that they correspond to;

  • 1-Pawn
  • 2-Bishop
  • 3-Knight
  • 5-Rook
  • 6-Queen
  • 10-King

The negative of the same numbers correspond to the black pieces!

This is the code for generating all moves of the knight.

                if (row > 0 && col < 6 && board[row-1][col+2] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row-1);
                    move.push_back(col+2);
                    pseudomoves.push_back(move);
                    move.clear();
                }
                if (row > 1 && col < 7 && board[row-2][col+1] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row-2);
                    move.push_back(col+1);
                    pseudomoves.push_back(move);
                    move.clear();
                }
                if (row < 7 && col < 6 && board[row+1][col+2] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row+1);
                    move.push_back(col+2);
                    pseudomoves.push_back(move);
                    move.clear();
                }
                if (row < 6 && col < 7 && board[row+2][col+1] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row+2);
                    move.push_back(col+1);
                    pseudomoves.push_back(move);
                    move.clear();
                }
                if (row < 6 && col > 0 && board[row+2][col-1] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row+2);
                    move.push_back(col-1);
                    pseudomoves.push_back(move);
                    move.clear();
                }
                if (row < 7 && col > 1 && board[row+1][col-2] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row+1);
                    move.push_back(col-2);
                    pseudomoves.push_back(move);
                    move.clear();
                }
                if (row > 1 && col > 0 && board[row-2][col-1] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row-2);
                    move.push_back(col-1);
                    pseudomoves.push_back(move);
                    move.clear();
                }
                if (row > 0 && col > 2 && board[row-2][col-1] <= 0){
                    move.push_back(row);
                    move.push_back(col);
                    move.push_back(row-2);
                    move.push_back(col-1);
                    pseudomoves.push_back(move);
                    move.clear();
                }
}
            
\$\endgroup\$
3
  • 4
    \$\begingroup\$ Welcome to code review. This question currently has 3 votes to close because it is a snippet of code and not even a full function. We have no context on how the code is being used by the class or the program. To help optimize the code we need to have a good idea of how the code is being used. \$\endgroup\$
    – pacmaninbw
    Aug 9, 2020 at 22:44
  • 2
    \$\begingroup\$ While the purpose of the program was different, you might want to looks at KMMoveFilters.cpp in my third question on code review. \$\endgroup\$
    – pacmaninbw
    Aug 9, 2020 at 22:57
  • \$\begingroup\$ Okay i am sorry for that, i just joined this community i didn't know i have to post the full code \$\endgroup\$
    – user228914
    Aug 10, 2020 at 5:28

1 Answer 1

3
\$\begingroup\$

Of course this can be improved. Look at all the repetition... then, factor it out into a function. You'll end up with something like this:

pseudomoves.push_back_if_legal(row, col, row-1, col+2);
pseudomoves.push_back_if_legal(row, col, row-2, col+1);
pseudomoves.push_back_if_legal(row, col, row+1, col+2);
pseudomoves.push_back_if_legal(row, col, row+2, col+1);
pseudomoves.push_back_if_legal(row, col, row+2, col-1);
pseudomoves.push_back_if_legal(row, col, row+1, col-2);
pseudomoves.push_back_if_legal(row, col, row-2, col-1);
pseudomoves.push_back_if_legal(row, col, row-2, col-1);

As a bonus, it is now relatively obvious where the bug is!

\$\endgroup\$
4
  • \$\begingroup\$ I am looking to decrease the time taken to run it. I wanted to know if there was anything to improce in the LOGIC part. \$\endgroup\$
    – user228914
    Aug 9, 2020 at 15:49
  • \$\begingroup\$ Also , pseudo moves is a vector of vectors. This means i need to make another vector. Add the values. Then add it to pseudomoves. \$\endgroup\$
    – user228914
    Aug 9, 2020 at 15:51
  • 3
    \$\begingroup\$ "Make it correct; then make it fast." Your way of writing it failed to make it correct. I offer you a better path. :) \$\endgroup\$ Aug 9, 2020 at 16:20
  • \$\begingroup\$ :), Thanks for this! I will implement it. \$\endgroup\$
    – user228914
    Aug 9, 2020 at 16:29