HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Custom component organization system

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
customsystemorganizationcomponent

Problem

I'm rewriting an old Swing project that I did a few months ago, a fairly simple copy-cat of MS paint. I've been doing a pretty good job of keeping the code maintainable and organized, but I've hit a wall on one class. All I need is some general tips on organization on this one class.

The way I have my program set up is that each component of my painting program (buttons for colors, eraser button, JPanel to paint on) is actually a relative (extended from) of another swing component class (for instance PaintPanel, the panel you paint on, extends JPanel, or colorButton is an extension of JButton). This just allows me to keep things encapsulated and organized.

So, I needed a JFileMenu for the top of the program in order to save, open files, edit, etc. I created a new Class called PaintMenuBar, extended it from JMenuBar, and started creating the JMenus and JMenuItems. Now, each JMenuItem obviously needs an ActionListener, so I just gave each one a different ActionListener. I'm skeptical about this because I know it can be kind of disorganized looking.

Basically, I just want somebody to let me know if this practice is all right, or if there is a better alternative that doesn't involve me making 20 new classes.

Here's the entire class:

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

import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;

public class PaintMenuBar extends JMenuBar
{
private static final long serialVersionUID = 1L;

JMenu fileMenu;
JMenu editMenu;
JMenu aboutMenu;

public PaintMenuBar()
{
setUpFileMenu();
setUpEditMenu();
setUpAboutMenu();
}

// Set up menu items & such

private void setUpFileMenu()
{
fileMenu = new JMenu("File");

// File menu items
JMenuItem newMenuItem = new JMenuItem("New");
newMenuItem.addActionListener(new ActionListener()
{
@Overr

Solution

+1 to @Gilbert Le Blanc's comment. Creating

I'd consider extracting out methods like createOpenMenuItem(), createSaveMenuItem(), createPrintMenuItem() for better readability and higher abstraction:

private JMenuItem createOpenMenuItem() {
    JMenuItem openMenuItem = new JMenuItem("Open");
    openMenuItem.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent e) {

        }
    });
    return openMenuItem;
}


Then the setUpFileMenu would be a lot cleaner:

private void setUpFileMenu() {
    fileMenu = new JMenu("File");

    // File menu items
    fileMenu.add(createNewMenuItem());
    fileMenu.add(createOpenMenuItem());
    fileMenu.add(createSaveMenuItem());
    fileMenu.add(createSaveAsMenuItem());
    fileMenu.add(createPrintMenuItem());

    add(fileMenu);
}


A maintainer would be helpful, it's easier to get an overview what the method does (without the details) and you can still check them if you are interested in. For example, finding code of save menu is much faster than before.


Now, each JMenuItem obviously needs an ActionListener,
so I just gave each one a different ActionListener.
I'm skeptical about this because I know it can be kind of disorganized looking.

I've found this practice easier to work with than having one ActionListener for multiple components (with a switch-case inside).


Basically, I just want somebody to let me know
if this practice is all right, or if there is a better
alternative that doesn't involve me making 20 new classes.

Having a lot of classes doesn't hurt if they have clear responsibilties.

Code Snippets

private JMenuItem createOpenMenuItem() {
    JMenuItem openMenuItem = new JMenuItem("Open");
    openMenuItem.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent e) {

        }
    });
    return openMenuItem;
}
private void setUpFileMenu() {
    fileMenu = new JMenu("File");

    // File menu items
    fileMenu.add(createNewMenuItem());
    fileMenu.add(createOpenMenuItem());
    fileMenu.add(createSaveMenuItem());
    fileMenu.add(createSaveAsMenuItem());
    fileMenu.add(createPrintMenuItem());

    add(fileMenu);
}

Context

StackExchange Code Review Q#30334, answer score: 2

Revisions (0)

No revisions yet.