Return to Answer

• 1

• 25.5k
• 7
• 59
• 141

Okay the Layouting earns itself a whole answer after I've seen it in much detail...

###Eager initialization:

This is somewhat a continuation of my earlier answer, where I already criticised this.

You can speed this up significantly by actually going over all the members referenced in your initFrame() (which I inlined to the constructor for the reasons given in the other answer), and just making them eagerly initialized.

With a little reordering that even works just fine.

###Component cohesion.

It's important for code-understanding to be allowed to "ignore" some things in the code. This is especially helpful when you work with multiple variables as in your frame setup.

Instead of:

JMenu mnFile = new JMenu("File");
JMenuItem mntmConnectToDB = new JMenuItem("Open a Database");
JMenuItem mntmDisconnectDB = new JMenuItem("Disconnect Database");
// skipping ~ 20 LoC
/*---------------------- Default settings ----------------------*/
mntmDisconnectDB.setEnabled(false);
dbChoices.setBounds(173, 29, 184, 20);
// and another 5 lines
mntmConnectToDB.addActionListener(new ActionListener() {
 public void actionPerformed(ActionEvent evt) {
 connectBtnActionPerformed(evt);
 }
});
// another 20 or even more and then finally 
menuBar.add(mnFile);
mnFile.add(mntmConnectToDB);
mnFile.add(mntmDisconnectDB);

wow... that's one UI component stretched across some 70 Lines of code and intertwined with at least 10 other components. Nobody can grasp that in their heads... nobody

pack your components together, finish one component, then do the next. This makes your code form cohesive blocks that refer to the same component, something like:

 JToolBar toolBar = new JToolBar();
 toolBar.setBounds(0, 0, 874, 23);
 JButton btnOpenDatabase = new JButton("Open Database");
 btnOpenDatabase.addActionListener(this::connectBtnActionPerformed);
 JButton btnCloseDatabase = new JButton("Close Database");
 btnCloseDatabase.addActionListener(e -> {});
 JButton btnExport = new JButton("Export");
 toolBar.add(btnOpenDatabase);
 toolBar.add(btnCloseDatabase);
 toolBar.add(btnExport);
 frame.getContentPane().add(toolBar);

and at this point it should jump in your face: Extract that into a method:

frame.add(buildToolbar());
// .. somewhere else:
private JToolBar buildToolbar() {
 // all that code above and then just
 return toolBar;
}

###Minor gimmicks:

• As mentioned in the JFrame Javadocs, jframe.add() is a shorthand for jframe.getContentPane().add(). You should use it.

• There is no need to bind a no-op ActionListener... remove btnCloseDatabase.addActionListener(...

• Hardcoding numbers is generally ... unflexible. You should strive to make your UI adapt to changes in it's size. Magic numbers all over your code don't help. Try to get rid of them

• txtCondition is not used anywhere besides the layout initialization. It doesn't need to be a field.

• Setting the LayoutManager to null on something should not be necessary. Get rid of that line.
  Similarly applies to locationRelativeTo...

###End result

After these changes and some others outlined in the previously mentioned answer, the first few lines of your class currently look like this:

public class MainView {
 private JFrame frame = new JFrame();
 private CardLayout cardLayout = new CardLayout();
 private JPanel dbPanel = new JPanel(cardLayout);
 private JPanel previewPanel = new JPanel();
 private MainController controller;
 private JSplitPane splitPane = new JSplitPane(JSplitPane.HORIZONTAL_SPLIT, dbPanel, previewPanel);
 private JComboBox<String> dbChoices = new JComboBox<>();
 
 public MainView() {
 frame.setTitle("DB to Excel");
 frame.setSize(890, 541);
 frame.setLocationRelativeTo(null);
 frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 frame.getContentPane().setLayout(null);
 frame.setJMenuBar(buildMenuBar());
 splitPane.setBounds(0, 62, 874, 379);
 splitPane.setOneTouchExpandable(true);
 splitPane.setDividerLocation(600);
 dbChoices.setBounds(173, 29, 184, 20);
 dbChoices.addActionListener(this::selectDBActionPerformed);
 Label lblSelectDB = new Label("Select Database:");
 lblSelectDB.setBounds(10, 29, 146, 22);
 JLabel lblEnterCondition = new JLabel("Enter condition: WHERE ");
 lblEnterCondition.setBounds(10, 452, 146, 20);
 JTextField txtCondition = new JTextField();
 txtCondition.setBounds(167, 452, 190, 20);
 txtCondition.setColumns(10);
 frame.add(splitPane);
 frame.add(buildToolbar());
 frame.add(lblSelectDB);
 frame.add(dbChoices);
 frame.add(txtCondition);
 frame.add(lblEnterCondition);
 }
// class continues here