Checking if an index is out of bounds

Question

For a simple project, I represent a two-dimensional grid as a one-dimensional array, with both row and column indices between 1 and gridHeight. When retrieving a cell from the grid, I first need to check if both row and column IDs are within allowed bounds.

public void checkIfAllowed(int row_or_column) {
    if ((row_or_column < 1) || (row_or_column > gridHeight)) {
        throw new IndexOutOfBoundsException("Row or column ID is not acceptable.");
    }
}

public boolean getCellById(int row, int column) {
    checkIfAllowed(row);
    checkIfAllowed(column);
    return grid[row * gridHeight + column];
}

Can it possibly be improved?

There are two things about this code that I don't particularly like:

• Somewhat repetitive code in getCellById.
• It asks for permission instead of forgiveness: first checks everything, and then does the action. I would prefer a try .. catch construction (something like "try to return a value and then catch IndexOutOfBoundsException), but would it really work for Java? The array dimension is defined during run time, and initial declaration for my grid is just private boolean[] grid;.

Answer 1

A quick solution would be to use absolute value and mod on the input by the grid size. That way you could put in any numbers and never get an Out Of Bounds exception.

grid[Math.abs(row)%gridHeight*gridHeight + Math.abs(column)%gridHeight]

Answer 2

Not sure if I've understood yout question correctly, but I assume, that your grid is a square grid (you test both, maximal high and width, as a lower than grid's height).

Referring to your first question: There are many options. You can make a method which takes two parameters at a time:

public void checkIfAllowed(int row, int column){
      boolean areCorrect = checkIfRowAllowed(row) && checkIfColAllowed(column);
      if(!areCorrect){
           throw new IndexOutOfBoundsException("Row or column ID is not acceptable.");
      }
}

private boolean checkIfAllowed(int row_or_column) {
    return (row_or_column < 1 || row_or_column > gridHeight);
} 

Now, a client is not forced to check each row and column in separate lines. Just an example:

public boolean getCellById(int row, int column) {
      try{    
          checkIfAllowed(row, column);
          return grid[row * gridHeight + column];

      } catch (IndexOutOfBoundsException e) {
           // log an error, or do something else
      }
}

But this exception handling seems strage for me. I would simply throw it in getCellById(int row, int col) and handle when this method is invoked.

Answer 3

I cannot see anything wrong, both with checking the arguments first and the little bit of repetitive code. However, there are two things that I think need fixing:

• The exception message is not helpful. It says "row or column" and neither mentions bounds nor actual value. That does not make debugging easy. Once you fix that, there will be no repetitive code anymore.

• More importantly: Your code seems to have a bug. You are disallowing row and column indexes of 0. That does not go well with the fact that your data is stored in grid which is an array. That effectively means the entire first row (gridHeight many elements in the array) are unused, but still need to be allocated. If you create the grid as new boolean[gridHeight * gridHeight] this would still lead to an ArrayIndexOutOfBoundsException although it is not detected by checkIfAllowed(). Rather subtract one from the indices and save on the array length.

Finally, avoid underscores in Java identifiers, or at least don't mix them with camel case.