Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Please help me to make more readable and simple code like this. What if I had 20 components? Is it right to organize code like this?

package newpackage.view;

import java.awt.GridLayout;
import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JPasswordField;
import javax.swing.JRadioButton;
import javax.swing.JTextField;

public class UserPanel extends JPanel{

    private JLabel idLbl;
    private JLabel nameLbl;
    private JLabel passwordLbl;
    private JRadioButton adminRad;
    private JRadioButton userRad;
    private JTextField idFld;
    private JTextField nameFld;
    private JPasswordField passwordFld;
    private JButton submitBtn;

    public UserPanel() {
        setLayout(new GridLayout(4,2));
        idLbl = new JLabel("ID");
        nameLbl = new JLabel("Name");
        passwordLbl = new JLabel("Password");
        adminRad = new JRadioButton("Admin");
        userRad = new JRadioButton("User");
        idFld = new JTextField();
        nameFld = new JTextField();
        passwordFld = new JPasswordField();
        submitBtn = new JButton("Submit");
        add(idLbl);
        add(idFld);
        add(nameLbl);
        add(nameFld);
        add(passwordLbl);
        add(passwordFld);
        add(adminRad);
        add(userRad);
        add(submitBtn);
    }
}
share|improve this question

3 Answers 3

up vote 3 down vote accepted

Depending on your application structure, there are several ways to organize this.
If you have only this window with small number of components, just use add(new Compenent(...)) as Roman Ivanov suggested.

If you have to communicate a bit and have more Frames and more logic behind it, you could pick some of this bullet points:

  • Do not extend JPanel, instead have a private JPanel attribute and deal with it (Encapsulation). There is no need to give access to all JPanel methods for code dealing with a UserPanel instance. If you extend, you are forced to stay with this forever, if you encapsulate, you can change whenever you want without taking care of something outside the class.
  • Use a create() method to init the GUI (or createAndGet() if you need a reference). In this way you make it clear what happens there. The GUI is created. It is not obvious if this happens in the constructor.
  • Some of your attribute names look like you want to do something with them later on. So you will probably need listeners. You can add a addListeners() method which takes care about all the listeners.
  • If your init method still has too much work inside, you could split it up (like createFirstPanel(), createUserField(), createPlots() and so on).
  • Think about a cleaning up method/way. Do you want to destroy the window, reuse the window, clear the window? Have a single instance, multiple instances? You can handle all the different cases inside the class if you use encapsulation (and probably inside the controller, depends on your application model).
  • Use good names. Do not abbreviate Label with Lbl. Write Label. It is better readable and with auto completion, no more work to write. I would suggest to put the type before the name, like jLabelId, jLabelName, jTextFieldName, ... This is more consistent.

Just to say it again: If it is just a small class and no one will touch it ever again, use the easiest approach possible. Do not code for a future which is not clearly visible.

share|improve this answer

Try to NOT add each UI component in Class fields. Keep then local if possible, as close to usage as possible. Too much class fields does not make class easy readable - as reader think that all of them are do matter.

JLabel and other components, content(text) of which will never be changed, are good candidates to be declared as local variable or without variable at all: add(new JLabel("text"));

share|improve this answer

Creating UIs programmatically in Swing is verbose and error prone. There are visual tools that let you create your layout in a WYSIWYG style, and autogenerate the code for you. Many years ago I used NetBeans for this, I don't know what's today's flavor, if you use Eclipse you may want to check out this set of plugins:

http://code.google.com/p/visualswing4eclipse/

Also, take a look at this question.

http://stackoverflow.com/questions/564156/visual-swing-in-eclipse

I would restrict the programmatic approach to relatively simple UIs like yours. For 20 components it would be really cumbersome without a design tool, there's no way around it.

share|improve this answer
    
I use it. To much programmers say the same thing about this UI designers. I see this code which it creates I don't like it –  Valery Dec 10 '12 at 5:57

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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