Thursday, October 6, 2011

UI classes should not implement event listener interfaces

When one does a rudimentary search for Swing tutorials, one finds approximately a zillion examples that recommend writing code like this:
import java.awt.Container;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JTextField;

/**
 * A frame that holds a button which really wants to be pressed.
 * 
 * @author <a href="http://paulgestwicki.blogspot.com">Paul Gestwicki</a>
 */
public class PressMeFrame extends JFrame implements ActionListener {

    /**
     * Start the application.
     * 
     * @param args
     *            command-line arguments (ignored)
     */
    public static void main(String[] args) {
        PressMeFrame frame = new PressMeFrame();
        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);
    }

    /** The uneditable text field into which we will put our output */
    private JTextField outputField;

    /**
     * Create an instance of this class.
     */
    public PressMeFrame() {
        outputField = new JTextField(20);
        outputField.setEditable(false);

        JButton button = new JButton("Press me");
        button.addActionListener(this);

        Container contentPane = getContentPane();
        contentPane.setLayout(new BoxLayout(contentPane, BoxLayout.Y_AXIS));
        contentPane.add(button);
        contentPane.add(outputField);
    }

    @Override
    public void actionPerformed(ActionEvent arg0) {
        outputField.setText("Thank you.");
    }

}


The problem I want to focus in on is that PressMeFrame implements ActionListener, but in truth, I can't stand looking at a class that extends JFrame when it doesn't have to. Do I really want to bind myself to JFrame right now? Not when I can just as easily make a reusable component that can be stuck into other things. Let me start this again...

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;

/**
 * A frame that holds a button which really wants to be pressed.
 * 
 * @author <a href="http://paulgestwicki.blogspot.com">Paul Gestwicki</a>
 */
public class PressMePanel extends JPanel implements ActionListener {

    /**
     * Start the application.
     * 
     * @param args
     *            command-line arguments (ignored)
     */
    public static void main(String[] args) {
        JFrame frame = new JFrame();
        frame.getContentPane().add(new PressMePanel());
        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);
    }

    /** The uneditable text field into which we will put our output */
    private JTextField outputField;

    /**
     * Create an instance of this class.
     */
    public PressMePanel() {
        outputField = new JTextField(20);
        outputField.setEditable(false);

        JButton button = new JButton("Press me");
        button.addActionListener(this);

        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));
        add(button);
        add(outputField);
    }

    @Override
    public void actionPerformed(ActionEvent arg0) {
        outputField.setText("Thank you.");
    }

}


That's a little better, and if you followed the first, you can follow the second. Now, to the heart of the matter. While this works, I argue that it is not right. When we say that PressMePanel implements ActionListener, we are making a bold statement: we are saying that every PressMePanel instance is designed to listen for action events. For example, we could make another class that uses PressMePanel, like this:

import javax.swing.BoxLayout;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;

/**
 * A panel for text entry.
 * 
 * @author <a href="http://paulgestwicki.blogspot.com">Paul Gestwicki</a>
 */
public class TextEntryPanel extends JPanel {

    /**
     * Start the application.
     * 
     * @param args
     *            command-line arguments (ignored)
     */
    public static void main(String[] args) {
        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setContentPane(new TextEntryPanel());
        frame.pack();
        frame.setVisible(true);
    }

    public TextEntryPanel() {
        setLayout(new BoxLayout(this, BoxLayout.X_AXIS));

        PressMePanel pressMePanel = new PressMePanel();
        JTextField textField = new JTextField(10);
        textField.addActionListener(pressMePanel);

        add(textField);
        add(pressMePanel);
    }

}


That "works," in that when we press enter in the text field—which fires an ActionEvent—the message comes up in the PressMePanel. However, it doesn't really make sense. The interaction logic of PressMePanel is internal. It's simply not designed to be used in this way, but because it implemented ActionListener, this is perfectly legal. Defensive coding practice dictates that we should prevent this kind of abuse if we can. We get much better encapsulation if, instead of having PressMePanel implement ActionListener, it creates its own delegate listeners. One easy way to do this is to create inner classes to handle it:

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;

/**
 * A frame that holds a button which really wants to be pressed.
 * 
 * @author <a href="http://paulgestwicki.blogspot.com">Paul Gestwicki</a>
 */
public final class PressMePanel extends JPanel {

    /**
     * Start the application.
     * 
     * @param args
     *            command-line arguments (ignored)
     */
    public static void main(String[] args) {
        JFrame frame = new JFrame();
        frame.getContentPane().add(new PressMePanel());
        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);
    }

    /** The uneditable text field into which we will put our output */
    private JTextField outputField;

    /**
     * Create an instance of this class.
     */
    public PressMePanel() {
        outputField = new JTextField(20);
        outputField.setEditable(false);

        JButton button = new JButton("Press me");
        button.addActionListener(new MyButtonListener());

        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));
        add(button);
        add(outputField);
    }

    /**
     * Listens for action events, posting a message to the {@link #outputField}
     * when this happens.
     * 
     * @author <a href="http://paulgestwicki.blogspot.com">Paul Gestwicki</a>
     */
    private final class MyButtonListener implements ActionListener {
        @Override
        public void actionPerformed(ActionEvent arg0) {
            outputField.setText("Thank you.");
        }
    }

}


I've added a private inner class, MyButtonListener, which has one job: listen for action events and update the message in outputField when that happens. This is nicely encapsulated, and by making the class private, I'm not exporting it for potential abuse by other parts of the system. (Notice that I've also made both classes final. In the spirit of defensive programming, if I'm not designing for extension, I should prevent it.) There's another design improvement to be made. By defining the MyButtonListener as a class, I can now make as many instances of it as I want. Yet, my semantic model is that there is really only one, and it's attached to the button. Notice that this is implied by the naming of the class. Let's use anonymous inner classes instead:

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;

/**
 * A frame that holds a button which really wants to be pressed.
 * 
 * @author <a href="http://paulgestwicki.blogspot.com">Paul Gestwicki</a>
 */
public final class PressMePanel extends JPanel {

    /**
     * Start the application.
     * 
     * @param args
     *            command-line arguments (ignored)
     */
    public static void main(String[] args) {
        JFrame frame = new JFrame();
        frame.getContentPane().add(new PressMePanel());
        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);
    }

    /** The uneditable text field into which we will put our output */
    private JTextField outputField;

    /**
     * Create an instance of this class.
     */
    public PressMePanel() {
        outputField = new JTextField(20);
        outputField.setEditable(false);

        JButton button = new JButton("Press me");
        button.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent arg0) {
                outputField.setText("Thank you.");
            }
        });

        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));
        add(button);
        add(outputField);
    }
}


Now, there is no doubt that this specific behavior is tied to the button. Of course, if I had a need to replicate this behavior, I could make a named class or an Action object, but we can cross that bridge when we get there. Better to solve the problem at hand than one that may or may not come up later. Moving to an anonymous inner class provides with another interesting refactoring opportunity. I like to have as few fields in my objects as I can. Now that the text field is only ever accessed or manipulated from the constructor, I can pull it into a local variable rather than an instance variable. The only piece of Java magic required for this is that we have to make the field final so that it can be read from the inner class:

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JTextField;

/**
 * A frame that holds a button which really wants to be pressed.
 * 
 * @author <a href="http://paulgestwicki.blogspot.com">Paul Gestwicki</a>
 */
public final class PressMePanel extends JPanel {

    /**
     * Start the application.
     * 
     * @param args
     *            command-line arguments (ignored)
     */
    public static void main(String[] args) {
        JFrame frame = new JFrame();
        frame.getContentPane().add(new PressMePanel());
        frame.pack();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setVisible(true);
    }

    /**
     * Create an instance of this class.
     */
    public PressMePanel() {
        final JTextField outputField = new JTextField(20);
        outputField.setEditable(false);

        JButton button = new JButton("Press me");
        button.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent arg0) {
                outputField.setText("Thank you.");
            }
        });

        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));
        add(button);
        add(outputField);
    }
}


Nice and tidy, but more importantly, our PressMePanel is well-encapsulated.

Now, textbook and tutorial authors, you can stop making every class under the sun implement listener interfaces.

You're welcome.

3 comments:

  1. Here here! Indeed unnecessary extension of containers and implementation of listeners is a downside of OOP. I'll be grafting on a component oriented approach for Swing soon in a similar manner as what I've done with Android. On Android of course I extend Application and Activity to add component manager functionality, but also View, LinearLayout, RelativeLayout too. These extensions are made final and one must interact with them via a component oriented manner. Each has an associated lifecycle system that is iterated over to provide functionality. Likewise with Swing when I get back to focusing on the desktop JFrame, JPanel will receive final overrides adding component manager functionality.

    I rewrote your example above to show dynamic injection / extraction of the standard GUI components into a generic panel.

    http://pastebin.com/hwXd62cG

    It could likely use better description of what is going on, but hopefully it's clear. Things get more interesting when one is creating custom components that have custom drawing in this manner as opposed to injecting premade Swing components.

    One aspect of my efforts is to minimize the listener pattern such that instead of implementing an interface the interface becomes a system and is embedded in a component manager creating a has-a relationship rather than is-a. This is how I handle input event distribution now in my efforts rather than simply implementing an input listener interface.

    ReplyDelete
  2. That's a nice example, Michael---thanks for sharing it!

    Something about it reminds me of Allen Holub's presentation of the Visual Proxy pattern for OO UI development in his Javaworld series (http://www.javaworld.com/javaworld/jw-07-1999/jw-07-toolbox.html). Briefly, encapsulation is achieved by having model objects generate their views, leaving the assembling of these views to a presenter a la MVP. It's unconventional but very interesting, and I've had some luck with it on small projects. If you've not already read the articles, you might enjoy them.

    ReplyDelete
  3. Cool; it's definitely the direction I'm taking for new GUI component creation. There were a couple of small copy/paste inaccuracies, but the idea is there on how to pull that off with my efforts:
    http://pastebin.com/xee7JdM9

    Without writing more there are a few things going on under the hood, but essentially "dynamicInject/Extract" gets called with a component / system gets added or removed.

    I'll check out the article as I'm an architecture nut of sorts, but am under the gun to get the early access of my efforts all together for AnDevCon II. I'll definitely keep you in the loop on when things are ready to check out as this should be soon.

    ReplyDelete