IF Statement Checking (Not Working Properly)

Question

randomEmpty() returns a random coordinate on the n x n grid that is empty (Method works). randomAdjacent() uses randomEmpty() to select an EMPTY coordinate on the map. Comparisons are then made to see if this coordinate has an VALID adjacent coordinate that is NON-EMPTY. The PROBLEM is that randomAdjacent does not always return the coordinates of space with an adjacent NON-EMPTY space. It will always return valid coordinates but not the latter. I can't spot the problem. Can someone help me identify the problem?

public int[] randomEmpty()
{
    Random r = new Random();
    int[] random = new int[2];
    int row = r.nextInt(array.length);
    int column = r.nextInt(array.length);
    while(!(isEmpty(row,column)))
    {
        row = r.nextInt(array.length);
        column = r.nextInt(array.length);
    }
    random[0] = row+1;
    random[1] = column+1;
    return random;        
}

public int[] randomAdjacent()
{
    int[] adjacentToX = new int[8];
    int[] adjacentToY = new int[8];
    int[] adjacentFrom = randomEmpty();
    int count;
    boolean isTrue = false;
    boolean oneAdjacentNotEmpty = false;

    while(!(oneAdjacentNotEmpty))
    {
        count = 0;

        if(validIndex(adjacentFrom,1,-1))
        {
            adjacentToX[count] = adjacentFrom[0]+1;
            adjacentToY[count] = adjacentFrom[1]-1;
            count++;
        }
        if(validIndex(adjacentFrom,0,-1))
        {               
            adjacentToX[count] = adjacentFrom[0];
            adjacentToY[count] = adjacentFrom[1]-1;
            count++;
        }
        if(validIndex(adjacentFrom,-1,-1))
        {          
            adjacentToX[count] = adjacentFrom[0]-1;
            adjacentToY[count] = adjacentFrom[1]-1;
            count++;
        }
        if(validIndex(adjacentFrom,-1,0))
        {        
            adjacentToX[count] = adjacentFrom[0]-1;
            adjacentToY[count] = adjacentFrom[1];
            count++;
        }
        if(validIndex(adjacentFrom,-1,1))
        {       
            adjacentToX[count] = adjacentFrom[0]-1;
            adjacentToY[count] = adjacentFrom[1]+1;
            count++;
        }
        if(validIndex(adjacentFrom,0,1))
        {         
            adjacentToX[count] = adjacentFrom[0];
            adjacentToY[count] = adjacentFrom[1]+1;
            count++;
        }
        if(validIndex(adjacentFrom,1,1))
        {         
            adjacentToX[count] = adjacentFrom[0]+1;
            adjacentToY[count] = adjacentFrom[1]+1;
            count++;
        }
        if(validIndex(adjacentFrom,1,0))
        {        
            adjacentToX[count] = adjacentFrom[0]+1;
            adjacentToY[count] = adjacentFrom[1];
            count++;
        }
        for(int i = 0; i < count; i++)
        {
            if(!(isEmpty(adjacentToX[i],adjacentToY[i])))  
            {
                oneAdjacentNotEmpty = true;
                isTrue = true;
            }
        }
        if(isTrue)
            break;
        else
            adjacentFrom = randomEmpty();           
    }
    return adjacentFrom;
}

public boolean validIndex(int[] a,int i, int j)
{
    try
    {
        Pebble aPebble = array[a[0]+i][a[1]+j];
        return true;
    }
    catch(ArrayIndexOutOfBoundsException e)
    {
        return false;
    }
}
public void setCell(int xPos, int yPos, Pebble aPebble)
{
    array[xPos-1][yPos-1] = aPebble;
}

public Pebble getCell(int xPos, int yPos)
{
    return array[xPos-1][yPos-1];
}

JUNIT Test Performed:

@Test
public void testRandomAdjacent() {
    final int size = 5;
    final Board board2 = new Board(size);
    board2.setCell(1, 1, Pebble.O);
    board2.setCell(5, 5, Pebble.O);
    int[] idx = board2.randomAdjacent();
    int x = idx[0];
    int y = idx[1];
    boolean empty = true;
    for (int i = x - 1; i <= x + 1; i++) {
        for (int j = y - 1; j <= y + 1; j++) {
            if ((i == x && j == y) || i < 1 || j < 1 || i > size || j > size) {
                continue;
            }
            if (board2.getCell(i, j) != Pebble.EMPTY)
                empty = false;
        }

    }
    assertFalse(empty);// NEVER gets SET TO FALSE
    assertEquals(Pebble.EMPTY, board2.getCell(x, y));
}

Answer 1

As for the answer: I got carried away optimizing your code for readability. I'd think it's most likely

if (board2.getCell(i, j) != Pebble.EMPTY)
            empty = false;

causing the problem as getCell operates in 1-based coordinates, but i, j are in 0-based.

You should think about your logic overall. The way I see it, your code might never terminate as randomEmpty() could keep returning the same field over and over again for an undetermined period of time.

I took the liberty to recode your if-if-if cascade into utility method easier to read:

public boolean hasNonEmptyNeighbor(int[] adjacentFrom) {
    for(int i = -1; i <= 1; ++i) {
      for(int j = -1; j <= 1; ++j) {
        if(validIndex(adjacentFrom, i, j) //Still inside the board
             &&                           // AND
           !isEmpty(adjacentFrom[0]+i     //not empty
                      ,adjacentFrom[1]+j)) {
          return true;
        }
      }
    }
    return false;
  }

Given my previous comment about random() being not the best of choices if you need to cover the full board, your main check (give me an empty cell with a non-empty neighbor) could be rewritten like this:

  public void find() {
     List<Point> foundPoints = new ArrayList<Point>();
     for(int i = 0; i < Board.height; ++i) { //Assumes you have stored your height
       for(int j = 0; j < Board.width; ++j) { //and your width
         if(isEmpty(i, j) && hasNonEmptyNeighbor(new int[]{i,j})) {
            //Found one.  
            foundPoints.add(new Point(i, j));
         }
       }
     }
     //If you need to return a RANDOM empty field with non-empty neighbor
     //you could randomize over length of foundPoints here and select from that list.
   }