Swing UI for database-to-Excel tool

I'm just wondering what is the cleanest way you can write your GUI/Swing code? Right below is the code I have for my main frame. Could you guys let me know if it looks clean/understandable, and if there is any way I can improve it?

The code I have provided is the main frame of my program, which is basically a GUI program that allows users to open different databases, select tables, and then the columns they would like to inner-join. I have a DatabaseView, and a TableView as well. This is my main view. I'm just wondering if I'm doing it properly (organization-wise). Like, if the ActionListeners are where they're supposed to be, how I grouped the customizers, and Actions, etc.

"Export" means exporting the selected tables' columns (inner-joined) to Excel.

```
public class MainView {
private JFrame frame;
private JPanel dbPanel;
private JPanel previewPanel;
private MainController controller;
private JSplitPane splitPane;
private JTextField txtCondition;
private JComboBox dbChoices;
private CardLayout cardLayout;

public MainView() {
try {
frame = new JFrame();
initFrame();
frame.setVisible(true);
UIManager.setLookAndFeel("com.sun.java.swing.plaf.nimbus.NimbusLookAndFeel");
}
catch (ClassNotFoundException | InstantiationException | IllegalAccessException
| UnsupportedLookAndFeelException e) {
// TODO Auto-generated catch block
e.printStackTrace();
}
}

private void initFrame() {
customizeFrame();

cardLayout = new CardLayout();
dbPanel = new JPanel(cardLayout);
previewPanel = new JPanel();

splitPane = new JSplitPane(JSplitPane.HORIZONTAL_SPLIT, dbPanel, previewPanel);
customizeSplitPane(splitPane);

dbChoices = new JComboBox();
txtCondition = new JTextField();

JMenuBar menuBar = new JMenuBar();

JMenu mnFile = new JMenu("File"

Minimize failure scopes

Try-Blocks should be as small as reasonably possible. Some people even advocate extracting error-handling into a separate method of the form:

void doSomething() {
    try {
        something();
    } catch (RelevantException | AnotherRelevantException e) {
        // handle appropriately
    }
}

as such the try-block in your constructor should be:

public MainView() {
        frame = new JFrame();
        initFrame();
        frame.setVisible(true);
        try {
          UIManager.setLookAndFeel("com.sun.java.swing.plaf.nimbus.NimbusLookAndFeel");
        }
        catch (ClassNotFoundException | InstantiationException | IllegalAccessException
                | UnsupportedLookAndFeelException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }

Constructors: dos and don'ts

Let's recapitulate the responsibilities of a constructor:

• Object initialization into a usable state

• ...

right. nothing else. A constructor is only responsible for getting the Object into a usable state. This means, no frame.setVisible(true); in the constructor.

Also it's often easier and clearer to eagerly initialize members of a class. This allows your constructor body to be laser focused on what you actually want to do.

so frame = new JFrame(); vanishes, too.

Also not in the responsibilities of a constructor: Setting global state. Modifications to the UIManager belong into your main-method. Don't abuse the constructor for that.

With all these changes your constructor shrinks considerably to:

public MainView() {
    initFrame();
}

Now what we could (and probably should do) is stop hiding the layouting setup in a separate method doing everything, but inline this monster (Eclipse has Alt+Shift+I) and then look at what we're doing there.

Layout initializations

OOP-Expert has written a nice section about that in their answer, I'd like to point out an alternative though.

While the separation of "components we need again" and "components we need to setup once" is important and useful, I'd like to stay away from lazy initialization.

You have these blocks of logically coherent steps in initFrame, you even marked them with comments.

These blocks are probably well off as extracted methods. I personally like to create the necessary component for usage in "factory-methods", if they don't need to be members of my class. For all others, I usually use some separate method to run a layout over them.

Again eager initialization is the key to not being suprised by things happening. You should also consider making as many members as possible final to not accidentally change them in your code.

For a more in-depth review of the layout initialization, check my other answer

Sidenote: This is all personal preference in here, YMMV

Use Lambdas and Method References

The block "Action Listeners" could be significantly shortened, when you use Lambdas, or even method references there. A good IDE will even tell you about opportunities to simplify your code that way.

mntmConnectToDB.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent evt) {
            connectBtnActionPerformed(evt);
        }
    });

could simply be:

mntmConnectToDB.addActionListener(this::connectBtnActionPerformed);

This reminds me: BtnActionPerformed is basically useless for all these methods. Don't name your methods after what they are, or when they are invoked but rather by what they do. That makes it easier to grasp what happens and usually is also more succinct.

Ending the Application

Stay away from System.exit to close your applications. That's a jerk move and you should only do it if you don't have any other choice.

Instead of shooting the whole JVM, you should enqueue a WINDOW_CLOSING Event, see how in this Stack Overflow post. This basically goes back down to your DefaultCloseOperation and runs that, instead of nuking the JVM (unless you say EXIT_ON_CLOSE, but eh :D)

StackExchange Code Review Q#114757, answer score: 5