0

I am trying to make cleaner code in my programs. So I was trying to compress my code to create buttons:

Before, I needed to copy this every single time:

Dimension JButton_Cryption_Size = JButton_Cryption.getPreferredSize();
        JButton_Cryption.setBounds(5, 5, JButton_Cryption_Size.width + 50, JButton_Cryption_Size.height);
        JButton_Cryption.setFocusPainted(false);

        JButton_Cryption.addActionListener(this);

        add(JButton_Cryption);

but now I made this method: (Don't pay attention to the button names, they are for testing)

public JButton  JButton_Testing1,
                    JButton_Testing2,
                    JButton_3;

    private void addJButton(JButton ButtonName, String Name, int x, int y, int width, int height, String ToolTip, boolean FocusedPainted, boolean Opaque, boolean ContentAreaFilled, boolean BorderPainted){
        ButtonName = new JButton(Name);

        Dimension Button_Size = ButtonName.getPreferredSize();
        if(width == 0){
            ButtonName.setBounds(x, y, Button_Size.width, height);
        }if(height == 0){
            ButtonName.setBounds(x, y, width, Button_Size.height);
        }if(width == 0 && height == 0){
            ButtonName.setBounds(x, y, Button_Size.width, Button_Size.height);
        }if(width != 0 && height != 0){
            ButtonName.setBounds(x, y, width, height);
        }

        ButtonName.addActionListener(this); // class: implements ActionListener

        ButtonName.setToolTipText(ToolTip);
        ButtonName.setFocusPainted(FocusedPainted);
        ButtonName.setOpaque(Opaque);
        ButtonName.setContentAreaFilled(ContentAreaFilled);
        ButtonName.setBorderPainted(BorderPainted);

        add(ButtonName);
    }

    private void addButtonToFrame(){
        addJButton(JButton_Testing1, "Testing 1", 150, 100, 172, 0, null, false, true, true, true);
        addJButton(JButton_Testing2, "Testing 2", 0, 0, 0, 0, null, false, true, true, true);
        addJButton(JButton_Testing3, "Testing 3", 200, 150, 250, 100, "YO", false, true, true, true);

    }

But when I want to add an action to the button, it wont work

@Override
    public void actionPerformed(ActionEvent e){
        Object src = e.getSource();
        if(src == JButton_Testing1){
            System.out.println("yo");
        }
    }

How can I make so I can keep my thing (or modify it a bit) so I can use the ActionListener correctly

3751_Creator
  • 656
  • 2
  • 8
  • 22
  • 1
    You have a refernce issue. The value of JButton_1 will never be assigned a value. It would be better to return the instance of the JButton you create and assign it to the variable you want. JButon_1 = addJButton(...);. It also looks like your using null layouts, which is a bad idea – MadProgrammer Mar 16 '14 at 09:08
  • @CrystalMeth I did say in the post that I implement ActionListener – 3751_Creator Mar 16 '14 at 09:10
  • You might want to take a look at [How to use actions](http://docs.oracle.com/javase/tutorial/uiswing/misc/action.html) – MadProgrammer Mar 16 '14 at 09:12
  • You may also want to take a look at the [builder pattern](http://www.oodesign.com/builder-pattern.html) – MadProgrammer Mar 16 '14 at 09:15

2 Answers2

7

Your question is about clean code, and you ask us to not pay attention to the button names. Half of having clean code is about having good names. Respect the Java conventions, and assign meaningful names to your variables and methods. Variables and methods start with a lowercase character in Java. And they don't contain underscores.

Also, Swing has layout managers. Stop setting bounds. Use layout managers.

Avoid having methods with 11 parameters.

Avoid having public fields. Fields should be private.

And finally, don't use this as the action listener. Use a separate class as your listener.

Regarding your problem: your addJButton() method doesn't add a listener to the button passed as argument. It ignores this argument, creates a new button, and adds the listener to this new button:

public void addJButton(JButton ButtonName, ...) {
    ButtonName = new JButton(Name);
JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • This code is in a test project, that is why I don't have great button names. They only serve as "Button Testing" purpose. Same with null layout... – 3751_Creator Mar 16 '14 at 09:11
  • 2
    Why should test projects use crappy names and contain unreadable code? Why not use correct names and write readable code instead? Good names and readable code saves time, whether the project is a test project or not. Your question starts with "I am trying to make cleaner code in my programs.". So do that: try to make cleaner code. – JB Nizet Mar 16 '14 at 09:13
  • 3
    @3751_Creator The right time to practice good practices is all the time, then it becomes habit and you don't need to switch between "modes" – MadProgrammer Mar 16 '14 at 09:16
  • There, added "correct names". As for why I never use a Layout, none of them suit really what I wan't. I've never found the most flexible one... – 3751_Creator Mar 16 '14 at 09:18
  • These are far from being correct names. Respect the Java naming conventions. – JB Nizet Mar 16 '14 at 09:24
  • What Layout do you recommend? – 3751_Creator Mar 16 '14 at 09:26
  • 1
    It all depends on how you want to lay out your components. Various layouts exist for a good reason. – JB Nizet Mar 16 '14 at 09:26
  • GroupLayout looks good but I don't understand why they make so ridiculosly hard to understand. That's why I am retisent – 3751_Creator Mar 16 '14 at 09:31
  • GroupLayout has been created explicitely for Matisse, which is the wysiwyg GUI designer in Eclipse, which generates the layout code for you. It indeed sucks (IMHO) when you create the code by yourself. If you're looking for an all-purpose layout manager, I would consider MIGLayout or JGoodies layouts (not part of the JDK). But using the layout managers of the JDK and composing complex panels from smaller ones using an appropriate layout manager is perfectly doable. – JB Nizet Mar 16 '14 at 09:37
  • *"I've never found the most flexible one..."* That is probably because 'the most flexible one' depends upon the requirement at that moment. That is where a [nested or compound layout](http://stackoverflow.com/a/5630271/418556) comes in handy. Of course, use layout padding & borders for [white space](http://stackoverflow.com/q/17874717/418556). – Andrew Thompson Mar 16 '14 at 10:06
  • 1
    *"GroupLayout has been created explicitely for Matisse, .. which generates the layout code for you. It indeed sucks (IMHO)"* Speaking of 'programmatically generated code', I like to use `GroupLayout` specifically for two columns of controls, the first being labels, the second being (single line) editing controls. E.G. as seen in [this answer](http://stackoverflow.com/a/21659516/418556). I cannot claim to be able to write `GroupLayout` code off the top of my head, but once it is in a factory method & debugged, it can be quite handy. – Andrew Thompson Mar 16 '14 at 10:13
1

A bad strategy is also the addButtonToFrame() method. It hard codes the number of buttons, their names, and everything. This way if you want to add one more button (for any reason) you have to write one more (custom) line of code to this method. The right way here is to make an addButtonToFrame(ArrayList <Button> buttons) method. You pass an ArrayList of (as many as you please) buttons in this method. Then add them in the panel, given the objects have been created with Dimension parameter.

But again, this is kind of making a new layout manager, and java has some really nice layout managers. In other words you are reinventing the wheel. That is not always bad (it is a good practice), but to make a good manager you have to spend time and (as I said) there are good managers.

Example:

class ButtonExample{
    ArrayList <JButton> buttons = new ArrayList<JButton>();
    ActionListener beh = new ButtonEventHandler()    //this is a custom class that contains actionPerformed() method

    createButtons(){
        for (int i = 0; i < buttons.size(); i++)
            buttons.get(i) = new JButton();
    }

    addListeners(){
        for (int i = 0; i < buttons.size(); i++)
            buttons.get(i).addActionListener(beh);
    }

}

The ArrayList is a kind of array without standard size. It is implemented using some nice tricks (that there is no need to know to use it) and you can access its objects with get() method (instead of [] operator like in regular arrays)

The addListeners() and createButtons() methods are dummies just to see how ArrayLists work. You can pass them as parameters in other methods the way you pass any regular object.

user2007447
  • 133
  • 1
  • 9
  • Ok, but what about the question about the ActionListener? – 3751_Creator Mar 16 '14 at 10:17
  • The ActionListener can also be included in the addButtonToFrame() method. For every button in the ArrayList you can add an ActionListener.The thing is that it is better to not use "this" as the Listener but an external class. This makes the code even more clean. – user2007447 Mar 16 '14 at 10:58
  • Could you add an example with the ArrayList please? I never realy worked with them. – 3751_Creator Mar 16 '14 at 14:42
  • added! Hope it helps! Also always look at the official Java documentation of the classes you are using. There are much more in the ArrayLists... http://docs.oracle.com/javase/6/docs/api/java/util/ArrayList.html – user2007447 Mar 16 '14 at 15:18