Function to test whether the source and destination squares represent a valid knight move The following is my solution to a problem. While it works - I want to learn how to optimize this in a much better way (I'm new to C++). My original idea was the use a map to map the r,c pairs and then test those but hoping someone can help optimize this for me.


  Write a function using a grid of string and two row/column pairs (r1, c1), (r2, c2) which returns true if there is a knight at chess board square (r1, c1) who can move to empty square (r2, c2). For your function to return true, there must be a knight at square (r1, c1), and the square at (r2, c2) must store an empty string, and both locations must be within the bounds of the grid.
  A knight makes an "L" shaped move, going 2 squares in one dimension and 1 square in the other. i.e. if the chess board (1, 2) stores "knight", then the call of knightMoves(board, 1, 2, 2, 4) returns true.


Here is my current solution

bool knightMoves(Grid&lt;string&gt;&amp; board, int r1, int c1, int r2, int c2);

int main() {

    //create a fake board
    Grid&lt;string&gt; board(7,7);
    board[1][2] = "knight";
    board[3][1] = "rock";
    board[0][4] = "king";

    //fire the function
    bool result = knightCanMove(board, 1, 2, 2, 4);
    cout &lt;&lt; result &lt;&lt; endl;

    return 0;
}

bool knightMoves(Grid&lt;string&gt;&amp; board, int r1, int c1, int r2, int c2) {

    bool canMove = false;
    string knight = "knight";

    //within grid bounds
    if(r1 &gt; board.numRows() || c1 &gt; board.numCols()) return false;
    if(r2 &gt; board.numRows() || c2 &gt; board.numCols()) return false;

    //test whether the co-ords match a knight string,
    //if yes, continue
    if (board[r1][c1] == knight) {
        if(board[r1 + 2][c1 + 1] == "" &amp;&amp; r1+2 == r2 &amp;&amp; c1+1 == c2) {
            canMove = true;
        }
        if(board[r1 + 2][c1 - 1] == "" &amp;&amp; r1+2 == r2 &amp;&amp; c1-1 == c2) {
            canMove = true;
        }
        if(r1 &gt;= 2 &amp;&amp; board[r1 - 2][c1 + 1] == "" &amp;&amp; r1-2 == r2 &amp;&amp; c1+1 == c2) {
            canMove = true;
        }
        if(r1 &gt;= 2 &amp;&amp; board[r1 - 2][c1 - 1] == "" &amp;&amp; r1-2 == r2 &amp;&amp; c1-1 == c2) {
            canMove = true;
        }
        if(board[r1 + 1][c1 + 2] == "" &amp;&amp; r1+1 == r2 &amp;&amp; c1+2 == c2) {
            canMove = true;
        }
        if(c1 &gt;= 2 &amp;&amp; board[r1 + 1][c1 - 2] == "" &amp;&amp; r1+1 == r2 &amp;&amp; c1-2 == c2) {
            canMove = true;
        }
        if(board[r1 - 1][c1 + 2] == "" &amp;&amp; r1-1 == r2 &amp;&amp; c1+2 == c2) {
            canMove = true;
        }
        if(c1 &gt;= 2 &amp;&amp; board[r1 - 1][c1 - 2] == "" &amp;&amp; r1-1 == r2 &amp;&amp; c1-2 == c2) {
            canMove = true;
        }
    }
    return canMove;
}
It looks like there may be an issue in range-checking.


You don't check for the rows and columns to be non-negative.
( &lt; 0 ) The types aren't marked as unsigned, so they might be negative.
If board is zero-based, and it seems to be, checking against numXYZ should include the boundary.
( &gt;= board.numXYZ() )


The range check becomes:

if ( r1 &lt; 0 || r1 &gt;= board.numRows() ||
     r2 &lt; 0 || r2 &gt;= board.numRows() ||
     c1 &lt; 0 || c1 &gt;= board.numCols() ||
     c2 &lt; 0 || c2 &gt;= board.numCols() ) {
  return false;
}


The jump check can be simplified. The absolute distance in one dimension must be one, and in the other must be two.

unsigned int rowdiff = abs(r1 - r2);
unsigned int coldiff = abs(c1 - c2);
if ( rowdiff == 1 &amp;&amp; coldiff == 2 || rowdiff == 2 &amp;&amp; coldiff == 1 )


Alternatively, we can note that the product of the distances must be ±2, leading to a more concise check, but that may obscure the underlying objective:

if ( abs((r1 - r2) * (c1 - c2)) != 2 ) return false;


At which point the final check is:

return board[r1][c1] == "knight" &amp;&amp; board[r2][c2] == "";