A far better way to use the index is with the % operator.
. . . colors[index++ % colors.length] . . .
You can reduce the entire method toYou might have problems when you have clicked 2147483647 times with arithmetical overflow, however. Try index++ and ++index, see which works better. You can divide it into 3 statements if you prefer:
index++; index %= colors.length; myButton.setBackground(colors[index]);
And colors is identical for all instances, so it could beneficially be a static field.
Umm.. can you elaborate on "clunky" please? Is it because of the COLOR_LIST.get(index)?
I had thought: The color array is static. So compile time memory allocation would help a bit in optimization. No chance of overflow. No pre/post incremental confusion (I personally still get confused in this sometimes Of course could be optimized to if it is going to result in a few less CPU cycles.
Joined: Oct 13, 2005
The most inefficient part is the linear search through your List for the index.
You set up index as an instance field, in which case it defaults to 0; Then you can go through the List and get the colour: