action listeners and event sources in Swing

Question

OK, so if I add an ActionListener to a GUI element, and it's the only element I use that ActionListener with, does it matter which of the following lines (a,b) I use to get the checkbox selected state?

final JCheckBox checkbox = (JCheckBox)this.buildResult.get("cbDebugTick");
checkbox.addActionListener(new ActionListener() {
 @Override public void actionPerformed(ActionEvent event){    
   boolean bChecked =
   // (a) checkbox.isSelected();
   // (b) ((JCheckBox)event.getSource()).isSelected();
   model.setPrintDebugOn(bChecked);
  }
});

It makes sense to me that if I add the ActionListener object to multiple GUI elements, then I should use (b).

And in (b), is it OK to blindly cast event.getSource() to JCheckBox, since I'm the one who added the action listener, or should I program defensively and do an instanceof check?

note: this question is in the context of event listeners in general; kdgregory has some good points below specifically re: checkboxes which I had neglected to consider.

Answer 1

I'd program with b defensively as it's the best-practice option. But if only you are ever going to use the code then there is no reason why you can't do a. However, imagine how happy you will be with yourself if you come back to it at some future point, change something and find you wrote good code which you can directly reuse...

Answer 2

in (b) to be rigourous, you should indeed do a instanceof check, but it's not that important. I would think both these lines are fine and acceptable, though (b) would be "better code"

Although, what is usually done in an action listener is simply call another method customized to your checkbox. So it would look like something like this:

 @Override public void actionPerformed(ActionEvent event) {                                  
    //your treatment would be in this method, where it would be acceptable to use (a)                  
    onCheckBoxActionPerformed(event)
}

Answer 3

I'd do neither.

If clicking the checkbox is going to start some action, I'd attach an ItemListener, then just look at the selection state in the ItemEvent.

However, checkboxes don't normally invoke actions, they manage state. So a better approach is to examine all of your checkboxes in response to whatever does kick off the action.

---

Edit: some commentary about the larger issues that the OP raised.

First, it's important to realize that large parts of Swing represent implementation convenience rather than a coherent behavior model. JCheckBox and JButton have nothing in common other than the fact that clicking within their space is meaningful. However, they both inherit from AbstractButton, which provides implementation details such as the button's label. It also assumes that buttons are "pressed", and that pressing a button will initiate some meaningful behavior (the action). In the case of JCheckbox, however, the button press is not important, the change in state is. That state change is signaled to the ItemListener -- which is also defined on AbstractButton even though state changes are meaningless to other button types (the JavaDoc even says "checkbox").

One of the things that Swing did get right -- if hard to use -- is the idea of that an Action is separate from the control initiating that action. An Action object can be invoked from multiple controls: a menu item, a pushbutton on a dialog, a keystroke, whatever. More important from a design perspective is that it takes you away from the idea of a generic "listener" that tries to figure out what needs to happen. I've seen apps where a single listener receives input from the entire menu system, for example, and then runs through a big if/else chain to figure out which menu item was pressed. Using Actions means you have more classes, but in the long run gives you a more maintainable app.

Finally, from a usability perspective, there's a difference between controls that maintain state, such as JCheckbox and JTextArea, and those that initiate actions, such as JButton and JMenuItem. I have seen a (web) app where clicking on a radio button takes you to a different page. That's bad. Even if you're planning to use listeners internally, to update the state of some model, you should ask yourself why the collection of GUI elements do not in themselves provide you with a model.

Answer 4

For the case where the listener is exclusive (such as an anon listener), I use (a).

If the listener will be reused (eg, this is an instance of ActionListener) I'll write it as:

@Override
public void actionPerformed(ActionEvent event) {
    Object src = event.getSource();
    if (src == checkbox) {
        boolean bChecked = checkbox.isSelected();
        // ...
    }
}

If you have several checkboxes and they are processed the same way, then instanceof makes sense.