Java: Preventing array going out of bounds.

Question

I'm working on a game of checkers, if you want to read more about you can view it here; http://minnie.tuhs.org/I2P/Assessment/assig2.html

When I am doing my test to see if the player is able to get to a certain square on the grid (i.e. +1 +1, +1 -1 .etc) from it's current location, I get an java.lang.ArrayIndexOutOfBoundsException error.

This is the code I am using to make the move;

public static String makeMove(String move, int playerNumber)
   {
       // variables to contain the starting and destination coordinates, subtracting 1 to match array size
       int colStart = move.charAt(1) - FIRSTCOLREF - 1;
       int rowStart = move.charAt(0) - FIRSTROWREF - 1;
       int colEnd   = move.charAt(4) - FIRSTCOLREF - 1;
       int rowEnd   = move.charAt(3) - FIRSTROWREF - 1;


       // variable to contain which player is which
       char player, enemy;
       if (playerNumber==1)
        {
            player= WHITEPIECE;
            enemy=  BLACKPIECE;
        }
       else
        {
            player= BLACKPIECE;
            enemy=  WHITEPIECE;
        }

        // check that the starting square contains a player piece
        if (grid [ colStart ] [ rowStart ] == player)
        {
            // check that the player is making a diagonal move
            if (grid [ colEnd ] [ rowEnd ] == grid [ (colStart++) ] [ (rowEnd++) ] &&
                grid [ colEnd ] [ rowEnd ] == grid [ (colStart--) ] [ (rowEnd++) ] &&
                grid [ colEnd ] [ rowEnd ] == grid [ (colStart++) ] [ (rowEnd--) ] &&
                grid [ colEnd ] [ rowEnd ] == grid [ (colStart--) ] [ (rowEnd--) ])
                {
                    // check that the destination square is free
                    if (grid [ colEnd ] [ rowEnd ] == BLANK)
                    {
                        grid [ colStart ] [ rowStart ] = BLANK;
                        grid [ colEnd ] [ rowEnd ]     = player;

                    }
                }
            // check if player is jumping over a piece
            else if (grid [ colEnd ] [ rowEnd ] == grid [ (colStart+2) ] [ (rowEnd+2) ] &&
                     grid [ colEnd ] [ rowEnd ] == grid [ (colStart-2) ] [ (rowEnd+2) ] &&
                     grid [ colEnd ] [ rowEnd ] == grid [ (colStart+2) ] [ (rowEnd-2) ] &&
                     grid [ colEnd ] [ rowEnd ] == grid [ (colStart-2) ] [ (rowEnd-2) ])
                 {
                   // check that the piece in between contains an enemy
                    if ((grid [ (colStart++) ] [ (rowEnd++) ] == enemy ) &&
                        (grid [ (colStart--) ] [ (rowEnd++) ] == enemy ) &&
                        (grid [ (colStart++) ] [ (rowEnd--) ] == enemy ) &&
                        (grid [ (colStart--) ] [ (rowEnd--) ] == enemy ))
                    {
                        // check that the destination is free
                        if (grid [ colEnd ] [ rowEnd ] == BLANK)
                        {
                            grid [ colStart ] [ rowStart ] = BLANK;
                            grid [ colEnd ] [ rowEnd ]     = player;
                        }

                    }
                 }

        }

I'm not sure how I can prevent the error from happening, what do you recommend?

Answer 1

The first thing that jumps to mind is your use of postincrement expressions like (colstart++) in the middle of an if statement condition. There are certainly cases where this might be useful but I don't believe yours is one of them.

Use (colstart+1) instead; it doesn't change the value of the colstart variable itself and it looks like that's what you really want to do.

In more detail, suppose colstart is 4:

System.out.println(colstart); // prints 4
System.out.println(colstart++); // prints 4
System.out.println(colstart); // prints 5

Compare to:

System.out.println(colstart); // prints 4
System.out.println(colstart+1); // prints 5
System.out.println(colstart); // prints 4

Answer 2

Add conditional statements to ensure that the values you are feeding to the array are between 0 and the size - 1

Answer 3

Whenever you check the contents of an array by index there are three ways to go:

1. "Know" that your index is within bounds
2. Check beforehand that index is within bounds
3. Handle the error after you accessed an index out of bounds

Thus, for every dimension of your grid and every value for an index check beforehand or handle the error.

I agree with Greg, that postincrements are generally asking for trouble, although I see the merits here. You might want to check the postincrements:

// check that the player is making a diagonal move
        if (grid [ colEnd ] [ rowEnd ] == grid [ (colStart++) ] [ (rowEnd++) ] &&
            grid [ colEnd ] [ rowEnd ] == grid [ (colStart--) ] [ (rowEnd++) ] &&
            grid [ colEnd ] [ rowEnd ] == grid [ (colStart++) ] [ (rowEnd--) ] &&
            grid [ colEnd ] [ rowEnd ] == grid [ (colStart--) ] [ (rowEnd--) ])

Starting with some hypothetical point (2,2):

colStart = 2 -> 3 -> 2 -> 3 => 2 rowEnd = 2 -> 3 -> 4 -> 3 => 2

So, starting from (2,2) you are checking (3,3), (2,4) and (3,3) again, finishing with the original value. This seems highly unwanted to me, because you are doing the same test twice. (Could be that you meant to hit other spots.)