If statement ignores return method, other solutions?

Question

I have a class called Cells and in it I have an update method that runs this piece of code:

if(goalReached){
  if(returnNearestCell() > -1 && isTarget && this.checkCollide(cells.get(returnNearestCell()).x, cells.get(returnNearestCell()).y, cells.get(returnNearestCell()).mass)){
    addMass(cells.get(returnNearestCell()).mass);
    cells.get(returnNearestCell()).mass = 20;
    cells.get(returnNearestCell()).x = (int) Math.floor(Math.random() * 1001);
    cells.get(returnNearestCell()).y = (int) Math.floor(Math.random() * 701);
    isTarget = false;
  }
  if(returnNearestCell() > -1 && !isTarget){
    goalX = cells.get(returnNearestCell()).x; 
    goalY = cells.get(returnNearestCell()).y; 
    target = cells.indexOf(returnNearestCell());
    isTarget = true;

  }else if(returnNearestCell() == -1 ){ 
    goalX = (int) Math.floor(Math.random() * 1001);
    goalY = (int) Math.floor(Math.random() * 701);
    isTarget = false;
  }
  if(!isTarget){
    addMass(5);
  }
  goalReached = false;
}

Basically to sum it up, each cell looks for the nearest cell with a smaller mass and if a cell is found then set the goalX and goalY to the position of that cell. If no such cell with the same criteria is found then just go to a random position. The code works fine until for some reason the first if statement gets ignored:

returnNearestCell() > -1

Then I get an ArrayIndexOutOfBoundsException.

My returnNearestCell method goes as follows:

public int returnNearestCell(){

int x = 0;
int distance = 9999999;
int min = distance;

for(Cell cell : cells){
  if(this != cell){
    distance = (int)Math.sqrt((this.x - cell.x)*(this.x - cell.x ) + (cell.y - this.y)*(cell.y  - this.y));
    if(distance < min && this.mass > cell.mass + 10){
      min = distance;
      x = cells.indexOf(cell);
    }else if(distance < min && this.mass < cell.mass + 10 && cell.cellCount == cells.size()){
      x = -1;
    }
  }
}

return x;
}

This method returns the index of the cell with the criteria or -1. My question is: Is there any way to avoid this OutofBoundsException? I've tried multiple methods like doing a double check but I still get the same issue.

I suggest you do appropriate diagnostic work so you can narrow this down into a [mcve]. Given that there's behaviour you don't understand (the supposed "ignoring" of the `if` statement) I would try to work that out first. — Jon Skeet, Feb 13 '16 at 15:55
I'd also suggest calling `returnNearestCell` *once* in your method, and using that result throughout your method. Why would you want to call it that many times? I'd probably actually make it return the `Cell` (or null if nothing is found) rather than the index, too... it'll make your code a lot simpler, I suspect... — Jon Skeet, Feb 13 '16 at 15:57
It can help to know at what line it is throwing the exception — Marco Altieri, Feb 13 '16 at 15:58
The exception is always thrown here: cells.get(returnNearestCell()).y = (int) Math.floor(Math.random() * 701); — BruceTheGoose, Feb 13 '16 at 15:59
Is there any way to store an object into a variable and access it through that variable? — BruceTheGoose, Feb 13 '16 at 16:01
You are *mutating the cell* and then call `getNearestCell()` again, expecting the result to be the same. — dhke, Feb 13 '16 at 16:01

dhke · Accepted Answer · 2016-02-13T16:09:43.850

cells.get(returnNearestCell()).mass = 20;
cells.get(returnNearestCell()).x = (int) Math.floor(Math.random() * 1001);
cells.get(returnNearestCell()).y = (int) Math.floor(Math.random() * 701);

Here, you are mutating the cell and then call returnNearestCell() again. Since that method now runs with the changed parameters, the return value is possibly different. Most importantly, you shift the cell along the x coordinate and it is then in a different position when evaluated by the next call to returnNearestCell().

You might want to look up non-atomic updates and concurrent modification for more on this topic.

Is there any way to store an object into a variable and access it through that variable?

Yes and it's the solution to your problem:

if (goalReached) {
   // retrieve nearest cell once before modification
   final int nearestCellIndex = returnNearestCell();
   if (nearestCellIndex > -1 && isTarget) {
       // save cell.
       final Cell nearestCell = cells.get(nearestCellIndex);

       if (this.checkCollide(nearestCell.x, nearestCell.y, nearestCell.mass)) {

            // remainder of your code
       }
   }
}

Note that it is probably preferable to have returnNearestCell() return Optional<Cell> or at least a Cell object directly. Same goes for checkCollide() which could simply take a Cell object as parameter.

What would my method return if no cell is found(if I return the cell directly)? — BruceTheGoose, Feb 13 '16 at 16:11
@BruceTheGoose If the return type is `Cell`, your only sane option is to return `null`. If using Java 8, you might want to consider using [`Optional`](https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html) and return `Optional.empty()`. — dhke, Feb 13 '16 at 16:13

If statement ignores return method, other solutions?

1 Answers1