CODEWITHSUNDEEP
javaarraysexception
Xenorosth
Xenorosth

Reputation: 132

Checking a grid, throwing out of bounds exceptions

Currently, I'm trying to check a grid cells for data in all cells around it. Up, left, down, right, and all diagonals. How can I use exception throwing so I don't have to individually code the sides and corners?

This is currently what I have. isIsAlive() simply checks to see if the cell is "active". Neighbors for a cell include all active cells around it.

    public void calcNeighbors() throws ArrayIndexOutOfBoundsException{

    int x =0;

    int y =0;
    int neighbors = 0;



    while(x < 9){
        while(y < 9){

            if(generation[x+1][y+1].isIsAlive()){
                neighbors++;

            }
             if(generation[x+1][y].isIsAlive()){
                neighbors++;
            }
              if(generation[x+1][y-1].isIsAlive()){
                neighbors++;
            }
               if(generation[x][y-1].isIsAlive()){
                neighbors++;
            }
                if(generation[x-1][y-1].isIsAlive()){
                neighbors++;
            }
                 if(generation[x-1][y].isIsAlive()){
                neighbors++;
            }
                 if(generation[x-1][y+1].isIsAlive()){
                neighbors++;
            }
                 if(generation[x][y+1].isIsAlive()){
                neighbors++;
            }
            y++;
        }
        x++;
        neighbors = 0;
    }
}

Upvotes: 1

Views: 2404

Answers (3)

Mario Rossi
Mario Rossi

Reputation: 7799

It's not recommended to use exceptions for this purpose. But if you insist, you could do it in the following way. First, define a method

public boolean isAlive(int x,int y) {
    try {
        return this.generation[x][y].isIsAlive() ;
    } catch(IndexOutOfBoundsException ex) {
        return false ;    //  Or whatever you want to be the default
    }
}

And then use isAlive(x+1,y+1) instead of generation[x+1][y+1].isIsAlive() and so on.

Additionally, my impression is that you are mistakenly declaring a local variable int neighbors = 0;. I say this because you keep setting it to 0 at the end, but you don't store it anywhere. Personally, I would define a field neighbors in whatever is the base class of generation and:

for(int x= 0 ; x < generation.length ; x++ ) {
    for(int y= 0 ; y < generation[x].length ; y++ ) {
        generation[x][y].neighbors= 0 ;
        for(int dx= -1 ; dx <= 1 ; dx++ ) {
            for(int dy= -1 ; dy <= 1 ; dy++ ) {
              if( ! ( dx == 0 && dy == 0 ) && isAlive(x+dx,y+dx) ) {
                  generation[x][y].neighbors++;
              }
        }
    }
}

My concern with so many ifs are 3: 1. It's very easy to make a mistake. 2. It will be time consuming (and error-prone) to add any other code inside all ifs. 3. The logic is easier to understand. Though you could also add a comment explaining that you are going to check neighbors, and that neighbors are all 8 cells where row or column is +1 or -1 the current cell.

Also, now that we have reduced the number of ifs, we could also inline the function above and write the following:

for(int x= 0 ; x < generation.length ; x++ ) {
    for(int y= 0 ; y < generation[x].length ; y++ ) {
        generation[x][y].neighbors= 0 ;
        for(int dx= -1 ; dx <= 1 ; dx++ ) {
            for(int dy= -1 ; dy <= 1 ; dy++ ) {
                try {
                    if( ! ( dx == 0 && dy == 0 ) && isAlive(x+dx,y+dx) ) {
                        generation[x][y].neighbors++;
                    }
                } catch(IndexOutOfBoundsException ex) {
                    //  Do whatever you want in this case
                }
            }
        }
    }
}

Now, without abusing exceptions (which is by far the most recommended), I'd say add a function

public boolean isValidNeighbor(int i,int j) {
    return 0 <= i && i < generation.length && 0 <= j && j < generation[i].length ;
}

And your code becomes:

for(int x= 0 ; x < generation.length ; x++ ) {
    for(int y= 0 ; y < generation[x].length ; y++ ) {
        generation[x][y].neighbors= 0 ;
        for(int dx= -1 ; dx <= 1 ; dx++ ) {
            for(int dy= -1 ; dy <= 1 ; dy++ ) {
                if( ! ( dx == 0 && dy == 0 ) && isValidNeighbor(x+dx,y+dx) && isAlive(x+dx,y+dx) ) {
                    generation[x][y].neighbors++;
                }
            }
        }
    }
}

Much, much better. And, even if not the main reason, less code and complexity than with exceptions!!!

Upvotes: 1

Hovercraft Full Of Eels
Hovercraft Full Of Eels

Reputation: 285405

Your list of if blocks is ugly (to be blunt) and dangerous. Instead use nested for loops but calculate the upper and lower bounds of the for loops taking the edges into consideration.

for (int x = 0; x < MAX_X; x++) {
  for (int y = 0; y < MAX_Y; y++) {

    int minRow = Math.max(0, x - 1);
    int maxRow = Math.min(MAX_X - 1, x + 1);
    int minCol = Math.max(0, y - 1);
    int maxCol = Math.min(MAX_Y - 1, y + 1);

    for (int row = minRow; row <= maxRow; row++) {
      for (int col = minCol; col <= maxCol; col++) {
         if (row != x || col != y) {
           if(generation[row][col].isIsAlive()){
             neighbors[x, y]++;
           }
         }
      }
    }
  }
}

Upvotes: 4

rgettman
rgettman

Reputation: 178263

You shouldn't throw your own exception if the hypothetical neighbor would be out of bounds. Java would throw an ArrayIndexOutOfBoundsException anyway.

You need to check your bounds before you access the array; don't access the array if either your x or y is out of range.

Upvotes: 3

Related Questions