Java: where should I put the anonymous logical code of the listener?

Question

we had a debate about what is the best use of listeners in java: should the listener logic remain in an anonymous class or should it be in a separate method, for example:

button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { // code here } }); 

or

 button.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent e) { buttonPressed(); } }); private void buttonPressed() { // code here } 

which is the recommended way in terms of readability and maintainability? I prefer to keep the code inside the listener, and only if it gets too big do it make it an inner class. Here I assume that the code is not duplicated anywhere.

Thanks.

Answer 1

My rule:

• If the listener has more than two methods, create a named class.
• If the listener spans more than 10 lines, create a named class.

Pretty simple, easy to use and more or less readable code. But then I have to admit that I didn’t even think about what your example shows.

Answer 2

This question partially answered here:

Is there a better practice for listeners

I also don't like the Anonymous Way for two reasons:
1) you cannot easily reuse the code so that you can find that you have duplicate code after a while. 2) I find it breaks the code reading (others disagree with personal taste). I think everyone will agree that if you do more than 5-10 lines, that an anonymous inner class is not a good idea (I would say that more than 2 is too much).

Answer 3

In my personal opinion, "it depends." If a listener needs to be added to only one component, it is very simple and is an integral part of the graphical interface, then an anonymous inner class will work well. If the listener is complex, it will be added to several components, it will have its own separate state, then a separate separate class would be better. Between them is a private inner class. NTN.

Edit: I feel bad. I answered another question: whether to use a separate autonomous class for the listener. Regarding whether to keep the internal code of the inline vs. class in the method, I agree with another poster, it will depend on the size and complexity of the listener code.

Answer 4

If the buttonPressed() method ever needs to get from anything other than an anonymous inner class, use the method. Otherwise, just put the code in actionPerformed() .

Answer 5

In my personal experience, it is better to follow the MVC pattern. Thus, there is a separate class representing the model that creates all the relevant actions, action listeners, etc. Preferably, the actions are represented as fields of the final classes that are created in the constructor.

For example:

 public class Model { private final Action buttonAction; ... public Model(final IController controller) { buttonAction = createButtonAction(controller); ... } private Action createButtonAction(final IController controller) { Action action = new Action("Action") { public void actionPerformed(final ActionEvent e) { // do the required action with long running ones on a separate Thread controller.run(); } }; action.set...// other initialisation such as icon etc ... return action; } ... public Action getButtonAction() { return buttonAction; } } 

The view is also represented by a separate class that takes the model as a constructor parameter, where the actual creation of the button takes place. For example:

 public class View extends JPanel { public View(final Model model) { ... JButton button = new JButton(model.getButtonAction(); ... } } 

With this approach, it is logical to implement the actionPerformed logic as part of an anonymous class, since it can be used very little repeatedly. All logic is encapsulated in the controller, so the actions really serve as a wrapper around the controller call, which will be used as the model for the button.

Answer 6

Yes, it depends a lot on what you are trying to do. I think anonymous inner classes got a bad rap because of two myths. One cannot reuse anonymous code. And two are a memory leak. But they are easily fixed by a simple approach. Save the link to the instance. To exchange code, simply create a link to an anonymous inner class.

 Action action = new AbstractAction("Open") {...}; JButton button = new JButton( action ); JMenuItem menuItem = new JMenuItem( action ); panel.getActionMap().put("openFile", action ); 

Now you can reuse this action for several components. For a later memory leak problem, you can use this link to cancel it, or the second and simpler option is WeakListeners. WeakListeners has an advantage that does not need to be managed after they are created.

As for the style, I think that anonymous listeners will be very convenient, and in some cases easier to read when working with threads in Swing, because it saves your code in one method (invokeLater, executeInBackground, etc.). When you listen to the anon listener in the instance method, I think that it separates the code in which you cannot read what happened before the listener, and the logic associated with the listener on one screen. They tend to be separated, and harder to follow.

Something to be aware of is that if you use ActionMaps, most memory leaks will go away by contacting you listening to the keyboard. Unfortunately, issues with focus listeners or listeners that are registered with central systems are still a problem. (Again, WeakListeners are wonderful here). And you already have a place to support the action with each component, so there is no need to create additional instance variables to store it. If you need to reuse two components (for example, in the menu bar and in the control), create a separate class.