ArrayIndexOutOfBoundsException when using loop for creating JFormattedTextField

I want to create a few JFormattedTextFields but always get ArrayIndexOutOfBoundsException and I do not understand why. The variable globalZaehler2 is 51 and then I will get the exception. But my loop says it has to be < field.length (which is 51). So globalZaehler2 can not be 51? Eclipse shows me the exception when I use ((JFormattedTextField)field[globalZaehler2]).selectAll();

for (globalZaehler2 = 0; globalZaehler2 < field.length; globalZaehler2++) {
    if (field[globalZaehler2] instanceof JFormattedTextField) {
        ((JFormattedTextField)field[globalZaehler2]).addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
((JFormattedTextField)field[globalZaehler2]).selectAll();
                // do something with the text in the field
            }
        });
    }
}
Jon Skeet
people
quotationmark

We can't tell terribly easily, but the name globalZaehler2 makes it sound like this is a field, not a local variable. Using a field for an "index" variable in a for loop is almost always a bad idea... I can't remember the last time I wanted the state of an object to change like this within a loop.

That means by the time actionPerformed executes, you could have executed the loop various other times, with different lengths of fields. If you try to make it a local variable, however, you'll come up with a different problem, because you're referring to it within the anonymous inner class - which means it would need to be a final local variable... and that goes to the real problem here.

Basically, you want the code in actionPerformed to use the value of globalZaehler2 for that iteration of the loop... but it's not executing during that iteration of the loop... it's executing when the action listener fires.

You could fix that by declaring a local variable within the loop:

for (globalZaehler2 = 0; globalZaehler2 < field.length; globalZaehler2++) {
    // In Java 8 you wouldn't need the final part
    final int copy = globalZaehler2;
    // Use copy instead of globalZaehler2 within actionPerformed
}

However, a far better solution would have two aspects:

  • When you're only using the variable in the for loop as an index into an array, and you're traversing the whole array, use an enhanced for loop
  • If you find yourself using a common subexpression multiple times in your code, extract that to a single expression

In your case, this would be:

// We don't know the type of field, so it's hard to guess the
// type here. Likewise I *would* use the name field, but that's the
// name of the array... it should probably be called fields, as it's
// naturally plural...
for (Object fooField : field) {
    if (fooField instanceof JFormattedTextField) {
        // Again, don't bother with final in Java 8
        final JFormattedTextField textField = (JFormattedTextField) fooField;
        textField.addActionListener(new ActionListener()) {
            @Override public void actionPerformed(ActionEvent e) {
                textField.selectAll();
                // etc
            }
        });
    }
}

Now you've naturally got a new local variable in the loop (textField) so it's fine for that to be effectively final, and to be used inside the actionPerformed method.

people

See more on this question at Stackoverflow