3

I am trying to develop a GUI for a Chess / Checkers game. When attempting to add ActionListeners to the buttons, Netbeans seems to give me a whole bunch of errors with suggestions that doesn't seem to solve the problem.

Here is the part of the code in question:

for (int i = 0; i < 8; i++) {
            for (int j = 0; j < 8; j++) {
                squares[i][j].addActionListener(new ActionListener() {
                    public void actionPerformed(ActionEvent e) {
                        if (!pressed) {
                            pressed = true;
                            fromR = i;
                        }
                        throw new UnsupportedOperationException("Not supported yet.");
                }
            });
        }
    }

squares[][] is the array all the buttons are stored in; the error occurs on the line fromR = i;

Is there a better way to add ActionListeners into buttons stored in arrays altogether?

CowNorris
  • 420
  • 2
  • 10
  • You can't assign to the variable `fromR` from within the innner class created for your `ActionListener`. – Elliott Frisch Nov 13 '15 at 22:44
  • Because `fromR` was probably created inside the method adding this anonymous class (so it needs to be final, or effectively final). I'm guessing `pressed` is an instance variable, so it works for this one. – Tunaki Nov 13 '15 at 22:45
  • And you can't use `i` either as it is not effectively final, – Paul Boddington Nov 13 '15 at 22:46
  • 1
    @ElliottFrisch Yes fromR is created inside the class, is there any ways to sidestep the issue? I'm trying to get the buttons to grab the co-ords when clicked. – CowNorris Nov 13 '15 at 22:48

1 Answers1

2

The problem is that you are referring to i inside the action listener and its continuously changing.

One option is to copy i to a new int like iValue here:

for (int i = 0; i < 8; i++) {
    for (int j = 0; j < 8; j++) {
        final int iValue = i;
        squares[i][j].addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                if (!pressed) {
                    pressed = true;
                    fromR = iValue;
                }
                throw new UnsupportedOperationException("Not supported yet.");
            }
        });
    }
}

This is clumsy though.

A cleaner alternative is to extract a method:

for (int i = 0; i < 8; i++) {
    for (int j = 0; j < 8; j++) {
        addActionListenerTo(squares[i][j], i);
    }
}

Here is that method:

private void addActionListenerTo(WhateverThisIs square, int i) {
    square.addActionListener(e -> {
        if (!pressed) {
            pressed = true;
            fromR = i;
        }
        throw new UnsupportedOperationException("Not supported yet.");
    });
}

Another alternative would be to have all the squares know their rank and file:

final class Square {
    final int rank;
    final int file:
    Square(int rank, int file) {
        this.rank = rank;
        this.file = file;
    }
}

Put those in a Collection and then you could do this:

squares.stream().forEach(square -> {
    square.addActionListener(e -> {
        if (!pressed) {
            pressed = true;
            fromR = square.rank;
        }
        throw new UnsupportedOperationException("Not supported yet.");
    });
});
Alain O'Dea
  • 21,033
  • 1
  • 58
  • 84
  • 1
    Thank you, that solves the problem. Why is this method clumsy & is there any better ways to sidestep this next time I run into this issue? – CowNorris Nov 13 '15 at 22:52
  • 1
    The clumsy part is in my solution: copying the variable. It just seems like appeasing the compiler rather than conscious design. – Alain O'Dea Nov 13 '15 at 22:57
  • @user2938375 I provide an alternative there using a separate method which cleans up the code. I also used a Lamba Expression to remove boiler-plate and reduce runtime cost. – Alain O'Dea Nov 13 '15 at 22:58