5
\$\begingroup\$

The goal of this is just to practice. I went off on a self-challenge to replicate something(So the names of the parts can be ignored), I'm not done yet (Though what is here is fully runnable), but have enough code that I thought it could merit taking a moment to examine whether I could be doing anything a cleaner way.

This is the main class:

import java.awt.BorderLayout;
import java.awt.GridLayout;
import java.awt.Font;

import java.io.File;
import java.io.FileNotFoundException;

import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JComponent;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;
import javax.swing.JPanel;
import javax.swing.JFileChooser;
import javax.swing.JScrollPane;
import javax.swing.JTabbedPane;
import javax.swing.JTable;
import javax.swing.UIManager;
import javax.swing.SwingUtilities;
import javax.swing.WindowConstants;
import javax.swing.table.TableModel;

public class EditorMain {
    JFileChooser fileChooser = new JFileChooser();

    EditorMain() {
        setUIFont (
            new javax.swing.plaf.FontUIResource("Verdana", Font.ITALIC, 11));

        JFrame frame = new JFrame();

        final int WIDTH = 1000;
        final int HEIGHT = 600;

        JPanel imagePanel = new JPanel();
        imagePanel.setLayout(new GridLayout(0, 3));
        imagePanel.add(new JButton(new ImageIcon("Assets/folder.png")));
        imagePanel.add(new JButton(new ImageIcon("Assets/save.png")));
        imagePanel.add(new JButton(new ImageIcon("Assets/printer.png")));


        JMenuBar menuBar = new JMenuBar();
        JMenu fileMenu = new JMenu("File");
        JMenu editMenu = new JMenu("Edit");
        JMenu helpMenu = new JMenu("Need Help?");
        JMenuItem openMenuItem = new JMenuItem("Open");
        JMenuItem cutMenuItem = new JMenuItem("Cut");
        JMenuItem copyMenuItem = new JMenuItem("Copy");
        JMenuItem pasteMenuItem = new JMenuItem("Paste");                                                                  

        openMenuItem.addActionListener(
            e -> {
                if (fileChooser.showOpenDialog(null) 
                    == JFileChooser.APPROVE_OPTION) {
                    File file = fileChooser.getSelectedFile();
                }
            }
        );

        fileMenu.add(openMenuItem);
        editMenu.add(cutMenuItem);
        editMenu.add(copyMenuItem);
        editMenu.add(pasteMenuItem);

        // add menus to menubar
        menuBar.add(fileMenu);
        menuBar.add(editMenu);
        menuBar.add(helpMenu);

        // put the menubar on the frame
        frame.setJMenuBar(menuBar);

        frame.setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);
        frame.setTitle("Trace Functional Method Specification Input Tool");
        frame.setBounds(0, 0, WIDTH, HEIGHT);
        frame.setLocationRelativeTo(null);
        frame.setLayout(new BorderLayout());
        frame.setVisible(true);


        final JTabbedPane tabbedPane = new JTabbedPane();

        JPanel panel = new JPanel();
        panel.setBounds(0, 0, WIDTH, HEIGHT);
        JTable table = new JTable(new ColorTableModel());

        table.setRowHeight(40);
        JScrollPane jscroll = new JScrollPane(table);
        jscroll.setBounds(0, 0, WIDTH, HEIGHT);
        table.setFillsViewportHeight(true); 
        panel.add(jscroll);
        panel.add(imagePanel);
        panel.setToolTipText("Example of helpful message!");

        ((JComponent) jscroll.getParent()).revalidate();
        tabbedPane.addTab("Input/Output Definition", panel);
        tabbedPane.addTab("Data Type Definition", new JPanel());
        tabbedPane.addTab("Access Methods and Event Descriptors Definition",
                new JPanel());
        tabbedPane.addTab("Auxiliary Functions Definition", new JPanel());
        tabbedPane.addTab("Output Behaviour Specification", new JPanel());

        JPanel header = new JPanel();
        header.setLayout(new GridLayout(2, 0));
        JLabel title = new JLabel("Module Interface Specification");
        title.setFont(new Font("Serif", Font.ITALIC, 20));
        header.add(title);
        header.add(imagePanel);

        tabbedPane.setBounds(0, 0, WIDTH, HEIGHT);
        frame.add(header, BorderLayout.PAGE_START); 
        frame.add(tabbedPane);
        frame.setIconImage(new ImageIcon("Assets/Icon.png").getImage());
        frame.setVisible(true);
    }

    public static void main(String[] args) {
        try {
            for (UIManager.LookAndFeelInfo info : 
                UIManager.getInstalledLookAndFeels()) {
                if ("Nimbus".equals(info.getName())) {
                    UIManager.setLookAndFeel(info.getClassName());
                    break;
                }
            }
        } catch (Exception e) {
            e.printStackTrace();
        }

        SwingUtilities.invokeLater(EditorMain::new);
    }

    public static void setUIFont(javax.swing.plaf.FontUIResource f) {
        java.util.Enumeration keys = UIManager.getDefaults().keys();
        while (keys.hasMoreElements()) {
            Object key = keys.nextElement();
            Object value = UIManager.get(key);
            if (value != null
                    && value instanceof javax.swing.plaf.FontUIResource)
                UIManager.put(key, f);
        }
    }
}

The ColorTableModel class:

import javax.swing.table.AbstractTableModel;

class ColorTableModel extends AbstractTableModel {
    Object rowData[][] = {
        { "", "", Boolean.TRUE, Boolean.FALSE },
        { "","", Boolean.FALSE, Boolean.TRUE }
    };

    String columnNames[] = { "Variable Name", "Data Type" , "Input", "Output"};

    public int getColumnCount() {
        return columnNames.length;
    }

    public String getColumnName(int column) {
        return columnNames[column];
    }

    public int getRowCount() {
        return rowData.length;
    }

    public Object getValueAt(int row, int column) {
        return rowData[row][column];
    }

    public Class getColumnClass(int column) {
        return (getValueAt(0, column).getClass());
    }

    public void setValueAt(Object value, int row, int column) {
        rowData[row][column] = value;
    }

    public boolean isCellEditable(int row, int column) {
        return true;
    }
}

I'd like a general review, with an emphasis on tidying my code/increasing readability.

\$\endgroup\$
  • 3
    \$\begingroup\$ A somewhat related github repository which is intended to help implementing filesystems that follow the nio.file api: github.com/fge/java7-fs-base \$\endgroup\$ – Vogel612 Mar 26 '15 at 21:07
7
\$\begingroup\$

EditorMain

import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JComponent;
// ...You get the idea.
import javax.swing.UIManager;
import javax.swing.SwingUtilities;
import javax.swing.WindowConstants;

At this point, you may as well just do the one * import statement:

import javax.swing.*;

Note that the TableModel import is not on the list of things I said to replace. It's unused, so you should get rid of it. Ditto for java.io.FileNotFoundException.

EditorMain() {

This should probably be public; you very rarely need to use package-private, and this doesn't seem like one of those cases. Especially since the class is public and contains main(String[] args).

You never seem to add click listeners to cut, copy, and pasteMenuItem. I suppose this is because you're in a basic version of the code, but at the very least, add a placeholder comment like

//TODO: Add listeners to these

between openMenuItem and the other three. It'll keep you from forgetting.

JScrollPane jscroll = new JScrollPane(table);

jscroll is a terrible name. Admittedly, there's really not a good name for a JScrollPane wrapper, but what I'd do is something like tableScroller, so it's at least clear what it's wrapped around.

} catch (Exception e) {

This is very often terrible idea. Always catch the most specific exceptions you can, with as many catch blocks as is reasonable -- in this case, you'll want to refer to the UIManager#setLookAndFeel Javadocs. They tend to list the exceptions (at least in my experience) in the same order that you should have list them when writing your try/catch blocks.

javax.swing.plaf.FontUIResource

You use the fully qualified name at least twice. Why not import it at the top?

SwingUtilities.invokeLater(EditorMain::new);

Nothing to criticize here. Just wanted to give you an internet pat-on-the-back for doing this. It's a very good idea to separate graphics and processing.

for (UIManager.LookAndFeelInfo info : UIManager.getInstalledLookAndFeels()) {
    if ("Nimbus".equals(info.getName())) {
        UIManager.setLookAndFeel(info.getClassName());
        break;
    }
}

This took me a couple of seconds to get and a read through the docs -- it might be worth it to add a comment along the lines of

// Use the theme called "Nimbus" if it's installed.

java.util.Enumeration is a raw type; it takes a generic, so you should use it with one. In this case, it'd just be java.util.Enumeration<Object>, but it'll help prevent errors later on if you add the generic now.

In openMenuItem.addActionListener, you never actually used file. I'm assuming that's for later, but I wanted to point it out anyway; I'd add a System.out.println(file.getAbsolutePath()); so you can see that it's the right file.

ColorTableModel

I'd recommend adding a check wherever you access an array with a value from a parameter. While it's not strictly necessary, since trying to call anything out-of-bounds will properly throw an error, getting an ArrayIndexOutOfBounds when it doesn't look like you're accessing an array can be confusing. It's as simple as wrapping it in a try/catch and throwing an IllegalArgumentException if you get an ArrayIndexOutOfBounds -- i.e. something like this:

// Same idea for all of them, so I'm just using `get` as an example.
public Object getValueAt(int row, int column) {
    try {
        rowData[row][column];
    } catch (ArrayIndexOutOfBoundsException e) {
        throw new IllegalArgumentException(String.format(Locale.ENGLISH, "(%s, %s) is an invalid location!", row, column), e);
    }
}

I used String#format because I personally find it nicer to use. Concatenation would work equally well.

public Class getColumnClass(int column) {
    return (getValueAt(0, column).getClass());
}

This should return a Class<?> to conform with AbstractTableModel.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.