GUI design in Java Swing

Question

I am developing a GUI using Swing, but before my project gets too complex. I would like to have a good design structure. I have a few ideas and will use a simple example to illustrate. Some feedback on whether or not this is a good approach would be great.

This is what my GUI looks like:

enter image description here

There are 2 Jpanels LeftPanel and RightPanel. When Show Name is clicked the name in that panel pops up.

My approach for this is:

1. If a component is used more than once, I write a class(GetNameButton,NameLabel).

2. I write a class for each panel. As the project gets bigger, the JFrame class won't get too cluttered and changes to a panel won't affect the JFrame class.

Here is the code:

public class MyFrame extends JFrame{
 public MyFrame(){
 this.setLayout(null);
 this.setBounds(100,100,400,400);
 this.add(new LeftPanel());
 this.add(new RightPanel());
 this.setVisible(true);
 }
 public static void main(String[] args) {
 new MyFrame();
 }
}

LeftPanel

public class LeftPanel extends JPanel implements ActionListener{
 CloseButton cb = new CloseButton();
 JLabel name = new JLabel("foo");
 GetNameButton gnb = new GetNameButton();
 public LeftPanel(){
 this.setLayout(null);
 this.setBounds(20, 20, 160, 320);
 this.setBorder(new SoftBevelBorder(BevelBorder.RAISED, null, null, null, null));
 this.add(new NameLabel());
 name.setBounds(50,10,40,10);
 this.add(name);
 this.add(cb);
 this.add(gnb);
 gnb.addActionListener(this);
 }
 @Override
 public void actionPerformed(ActionEvent e) {
 if(e.getSource()==gnb){
 JOptionPane.showMessageDialog(null, name.getText());
 }
 }
}

RightPanel

public class RightPanel extends JPanel implements ActionListener{
 JLabel name = new JLabel("bar");
 GetNameButton gnb = new GetNameButton();
 public RightPanel(){
 this.setLayout(null);
 this.setBounds(200, 20, 160, 320);
 this.setBorder(new SoftBevelBorder(BevelBorder.RAISED, null, null, null, null));
 this.add(new NameLabel());
 JLabel name = new JLabel("bar");
 name.setBounds(50,10,40,10);
 this.add(name);
 this.add(gnb);
 gnb.addActionListener(this);
 }
 @Override
 public void actionPerformed(ActionEvent e) {
 if(e.getSource()==gnb){
 JOptionPane.showMessageDialog(null, name.getText());
 }
 }
}

GetNameButton

public class GetNameButton extends JButton{
 public GetNameButton(){
 this.setText("Show Name");
 this.setBounds(20,100,120,20);
 }
}

NameLabel

public class NameLabel extends JLabel{
 public NameLabel(){
 this.setBounds(10,10,40,10);
 this.setText("NAME: ");
 }
}

CloseButton

public class CloseButton extends JButton implements ActionListener{
 public CloseButton(){
 this.setText("Close");
 this.setBounds(20,50,100,20);
 this.addActionListener(this);
 }
 @Override
 public void actionPerformed(ActionEvent e) {
 if(e.getSource()==this){
 System.exit(8);
 }
 }
}

• jliv902: I haven't done Java in years, but both panels seem to have a lot in common. Perhaps you can reduce duplicate code by putting all of the common features into a base class that extends JPanel.

Answer 1

• Variable names. Imagine yourself a couple of months from now on, when you look back at the code. Will you remember without looking at the variable declaration what gnb is short for? I suggest you use longer variable names so that you even won't have to try to remember it. getNameBtn would be a better name, and it's still not too long. Using closeBtn instead of cb is also better. cb could just as well mean clearbright or something else totally irrelevant.

• You seem to use setBounds to set the exact layout positions in pixels for your components. This is not recommended as this will not be flexible for resizing the windows. Instead use a proper LayoutManager such as BorderLayout and add some margins to make your buttons be placed approximately where they belong and still be flexible for if someone wants to resize the window. (Trust me, this can be more important than you think).

• You seem to extend a lot of JButtons and other things. Technically, what's different between a CloseButton and a GetNameButton? The only differences are their positions, labels and I suppose there will be a difference in what happens when you click them. All these things are properties, it does not warrant extension of the original class. Instead you can use a public static method that creates a JButton with the initialized properties of a "close-button" or a "get-name-button". (The same goes for NameLabel, it does not need to extend JLabel)

  Here's code for such a public static method:

  public static JButton createGetNameButton() {
   JButton btn = new JButton();
   btn.setText("Show Name");
   btn.setBounds(20,100,120,20);
   return btn;
  }
  

• jliv902 has a good point in his comment of putting common features into a base class for your panels. However, think about whether or not you really should extend them or not. Personally I think that extending a JPanel or a JFrame is more OK than extending a button, but I want you to know that there are even reasons for why a JFrame shouldn't be extended. Instead of extending, consider this:

  public class MyJPanelContainer implements ActionListener {
   private final JPanel panel;
   public MyJPanelContainer() {
   this.panel = new JPanel();
   // code to setup the panel the way you want it
   someComponent.addActionListener(this);
   }
   @Override
   public void actionPerformed(ActionEvent e) {
   // ...
   }
   public JPanel getPanel() { 
   // Technically, in another approach, you might not even need this getter, but that's a whole other story.
   return this.panel;
   }
  }
  

  If you would write a class like that, it will encapsulate the JPanel and thus hide several methods. The easiest effect to describe for using this approach is that it will drastically decrease the number of visible elements when you press Ctrl + Space, which will make it easier to see the method you are looking for.

• Shivan Dragon: While I agree with all your remarks, and specifically, with the first one as a general rule, I think in this particular case it's sensible to use a variable name of "cb" for a variable that's of type "CloseButton". From its type it's pretty clear what it does, replacing that by "CloseButton closeButton = new CloseButton()" looks kind of cluttery to me...
• Simon Forsberg: @ShivanDragon Sure, you would know what the button does if you know the type (as long as it's only one of them, what would you do if there were two?). However, you would only know the type if you read the line where it is declared. Also, I propose to not use a CloseButton type, therefore it would become JButton cb = createCloseButton(); which would make the variable name "cb" even worse.