Question

I've got 2 for loops, one nested inside of another. They loop through a 2D array of buttons to get the source of each button thats been clicked using the action listener.

When the button is found I pass the position/array indexs of the button to an external method. However when the button is found from the array of buttons the first for loop evaluates its termination condition to FALSE but still increments the value of i. Leading to an off by one error. My code is in the standard action performed method, with "event" being the ActionEvent. buttons[][] is a JButton array defined as an instance variable. It is of size 10 x 10 and is already added to the panel.

int i = 0; //this will loop through the columns in the array
int j  = 0; //loop through the rows
boolean locatedSource = false; //allows me to escape both loops

for(i = 0; !(locatedSource) && i < buttons.length; i++) //problem here, when i < buttons.length is FALSE i still gets incremented, leading to an off by one error
{
  for(j = 0; !(locatedSource) && j < buttons.length; j++)
  {
    if(event.getSource() == buttons[i][j])
    {
      locatedSource = true;
      break;
    }
  }
}
//do stuff with i and j in another method. Leads to array out of bounds error / off by one error
}

I should of mentioned, I'm not looking to solve this problem with the use of labels, they seem to be discouraged.

Was it helpful?

Solution

Explanation of the problem

The increment expression of a for loop is executed after each loop iteration not before. See the following quote from the Oracle Java tutorial:

The for statement provides a compact way to iterate over a range of values. Programmers often refer to it as the "for loop" because of the way in which it repeatedly loops until a particular condition is satisfied. The general form of the for statement can be expressed as follows:

for (initialization; termination;
     increment) {
    statement(s)
}

When using this version of the for statement, keep in mind that:

  1. The initialization expression initializes the loop; it's executed once, as the loop begins.
  2. When the termination expression evaluates to false, the loop terminates.
  3. The increment expression is invoked after each iteration through the loop; it is perfectly acceptable for this expression to increment or decrement a value.

For loop solution

You can re-write your loop so that the increment is the first statement inside the loop.

    for (i = 0; !(locatedSource) && i < buttons.length;) {
        i++;
        for (j = 0; !(locatedSource) && j < buttons.length;) {
            j++;
            if (event.getSource() == buttons[i][j]) {
                locatedSource = true;
            }
        }
    }

While Loop Version

Given that the loop variables are both initialised outside of the loop and you don't want to use a for-loop increment expression it might be clearer to rewrite the code to use while-loops as follows:

    while (!(locatedSource) && i < buttons.length) {
        i++;
        while (!(locatedSource) && j < buttons.length) {
            j++;
            if (event.getSource() == buttons[i][j]) {
                locatedSource = true;
            }
        }
    }

OTHER TIPS

Three possible solutions:

  1. Explicitely set a "found" index and do not reuse your for loop indices.
  2. Factor the searching out in an own method and return directly from the loop.
  3. Decrement i by 1 after finishing the loops.

Use some boolean flag set it in inner loop and check it in the beginning of outer loop.

Here is code:

    boolean found = false;
    for (i = 0; i < 10; i++) // problem here, when i < buttons.length is FALSE i still gets
                             // incremented, leading to an off by one error
    {
        if (found) {
            i--;
            break;
        }
        for (j = 0; j < 5; j++) {
            if (i == 5 && j == 3) {
                found = true;
                break;
            }
        }
        //if (found) {               
        //    break;
        //}
    }

Your code contains a comment "problem here, when i < buttons.length is FALSE i still gets incremented, leading to an off by one error", which is wrong in the ordering of the events.

First the cycle update block gets executed (like i++) and after that the condition is checked (like `i < buttons.length').

Meaning, that i == buttons.length is the correct state after the cycle ends without triggering the locatedSource condition.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top