Using .setVisible() outside the constructor breaks my GUI

Question

I am just now learning the basics of java GUI. I have this weird situation that I can't really explain.

I have a GUI class, where I build a simple JFrame. If I use .setVisible(true) within the constructor everything works fine, if I use it outside, nothing loads (the window is visible, but the buttons and what not aren't).

Why is this happening ?

public class GUI extends JFrame {


    private JTextField humanYears_TextField = new JTextField(3);
    private JTextField dogYears_TextField = new JTextField(3);
    private JButton convert_Button = new JButton("Convert");
    private JLabel greeting = new JLabel ("Dog years to Human years!");

    public GUI () {

        JFrame window = new JFrame();
        JPanel content = new JPanel();


        content.setLayout(new FlowLayout());
        content.add(this.greeting);
        content.add(new JLabel("Dog Years: "));
        content.add(this.dogYears_TextField);
        content.add(this.convert_Button);
        content.add(new JLabel("Human Years: "));
        content.add(this.humanYears_TextField);

        window.setContentPane(content);
        pack(); // aplica contentPane-ul
        window.setLocationRelativeTo(null);

        window.setTitle("Dog Year Converter");
        window.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        window.setVisible(true); // IF IT'S HERE IT WORKS
    }
}

public static void main(String[] args) {

    GUI dogYears = new GUI();
    //dogYears.setVisible(true); // IF IT'S HERE
                                 //NOTHING EXCEPT THE WINDOW LOADS
}

Why is this happening ?

In this example it doesn't matter, but what if I want to make a window visible only if I click a button or something ?

David Kroukamp · Accepted Answer · 2013-01-30T14:32:35.143

1) You create 2 instances of JFrame the first from extending JFrame on the class and the second from JFrame window=... You than go on to call setVisible(..) on the window and in your main you attempt to call setVisible(...) on your dogYears.

Solution

You should only create one JFrame per Swing GUI. Get rid of the extends JFrame (and the code that goes with it), as its not good practice to extend JFrame in Swing. Thus of course you wont be able to call setVisible(true) in your constructor which is fine either call it after creating the GUI in the constructor or make a method like setVisible in GUI class which will be a wrapper for your JFrames setVisble(boolean b) method.

Other suggestions

Always create your Swing components on the Event Dispatch Thread which you should do in via SwingUtilitites.invokeLater(Runnable r) block. Have a read on Concurrency in Swing.
Remember to call pack before setting JFrame visible and after adding components.
setLocationRelativeTo(..) should come after pack().
Better to use JFrame.DISPOSE_ON_CLOSE unless using Timers.
Also no need for setContentPane simply call add(..) on JFrame instance.

Here is your code with above mentioned fixes:

enter image description here

import java.awt.FlowLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.SwingUtilities;

public class GUI {

    private JTextField humanYears_TextField = new JTextField(3);
    private JTextField dogYears_TextField = new JTextField(3);
    private JButton convert_Button = new JButton("Convert");
    private JLabel greeting = new JLabel("Dog years to Human years!");
    private JFrame window = new JFrame();
    private JPanel content = new JPanel();

    public GUI() {
        content.setLayout(new FlowLayout());
        content.add(this.greeting);
        content.add(new JLabel("Dog Years: "));
        content.add(this.dogYears_TextField);
        content.add(this.convert_Button);
        content.add(new JLabel("Human Years: "));
        content.add(this.humanYears_TextField);

        window.add(content);

        window.setTitle("Dog Year Converter");
        window.setDefaultCloseOperation(JFrame.DISPOSE_ON_CLOSE);

        window.pack();
        window.setLocationRelativeTo(null);
        //window.setVisible(true); //works here now
    }

    //our wrapper method so we can change visibility of our frame from outside the class
    void setVisible(boolean visible) {
        window.setVisible(visible);
    }

    public static void main(String[] args) {
        //Create Swing components on EDT
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                GUI dogYears = new GUI();
                dogYears.setVisible(true);//works here too
            }
        });
    }
}

yeah, all were initially typed within minutes of each other +1 for expanding :-) — kleopatra, Jan 30 '13 at 12:55
@DavidKroukamp I just found this great answer and I've got one question, you said "create components on the EDT", so do I have to do all the work in Runnable or only for example calling the method setVisible(true)? — grape_mao, Apr 22 '14 at 21:44

score 5 · Answer 2 · edited Jan 30 '13 at 14:48

You've got two JFrames, the GUI class itself and the internal variable "window". Use only one.

Make sure that GUI does not extend JFrame (there's no need for this), and give the class a public void setVisible(boolean) method that sets the window to visible.

public class GUI {


    private JTextField humanYears_TextField = new JTextField(3);
    private JTextField dogYears_TextField = new JTextField(3);
    private JButton convert_Button = new JButton("Convert");
    private JLabel greeting = new JLabel ("Dog years to Human years!");
    private JFrame window = new JFrame();

    public GUI () {

        JPanel content = new JPanel();


        content.setLayout(new FlowLayout());
        content.add(this.greeting);
        content.add(new JLabel("Dog Years: "));
        content.add(this.dogYears_TextField);
        content.add(this.convert_Button);
        content.add(new JLabel("Human Years: "));
        content.add(this.humanYears_TextField);

        window.setContentPane(content);
        pack(); // aplica contentPane-ul
        window.setLocationRelativeTo(null);

        window.setTitle("Dog Year Converter");
        window.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        // window.setVisible(true); // IF IT'S HERE IT WORKS
    }

    public void setVisible(boolean visible) {
        window.setVisible(visible);
    }
}

+1 Oh HFOE never saw this answer when doing mine. great minds think alike :P. — David Kroukamp, Jan 30 '13 at 12:38
@Kalec: you are indeed correct that you will need a `pack()` method if this is not going to be called in the GUI class's constructor itself. — Hovercraft Full Of Eels, Jan 30 '13 at 14:23
@HovercraftFullOfEels your `setVisible(boolean visible)` would not compile as your `JFrame` - `window` is not declared within scope. Edited your code to reflect correct changes (move `JFrame` declaration to global) — David Kroukamp, Jan 30 '13 at 14:48

score 4 · Answer 3 · answered Jan 30 '13 at 12:22

4

You have two different instances :-)

in your constructor:

JFrame window = new JFrame();
window.setVisible(true); // IF IT'S HERE IT WORKS

in your main:

GUI dogYears = new GUI();
dogYears.setVisible(true); 
// dogYears != local var window in constructor

Which might get me starting on not extending classes, but using them.

answered Jan 30 '13 at 12:22

kleopatra

51,061
28
99
211

1

It got *me* started on that. 1+ – Hovercraft Full Of Eels Jan 30 '13 at 12:27
So then if I wanted to `setVisible` in, let's say `main`. I'd have to make a function `getWindow` ? Set *that* to visible ? Or a wrapper method ? – Kalec Jan 30 '13 at 12:30
1

Yes that may work work, but why is GUI extending JFrame if you do not use it as a JFrame, also see the EventDispatch posts and using SwingUtilitites.invokeLater(Runnable r) – Bruce Martin Jan 30 '13 at 12:36
lol +1 never saw all these posts when I started doing mine. +1 this is the problem – David Kroukamp Jan 30 '13 at 12:38

MC Emperor · Answer 4 · 2013-01-30T14:00:58.280

Your class GUI extends from a JFrame, but in the constructor, you are initializing another JFrame, which results in having two different JFrames.

Try this:

public class GUI {

    private JTextField humanYears_TextField = new JTextField(3);
    private JTextField dogYears_TextField = new JTextField(3);
    private JButton convert_Button = new JButton("Convert");
    private JLabel greeting = new JLabel ("Dog years to Human years!");

    public GUI () {
        JFrame window = new JFrame();
        JPanel content = new JPanel();

        content.setLayout(new FlowLayout());
        content.add(this.greeting);
        content.add(new JLabel("Dog Years: "));
        content.add(this.dogYears_TextField);
        content.add(this.convert_Button);
        content.add(new JLabel("Human Years: "));
        content.add(this.humanYears_TextField);

        window.setContentPane(content);
        pack(); // aplica contentPane-ul
        window.setLocationRelativeTo(null);

        window.setTitle("Dog Year Converter");
        window.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        window.setVisible(true);
    }

    // main

}

score 0 · Answer 5 · answered Jan 30 '13 at 12:38

you are creating an instance of JFrame in constructor of GUI which also extends JFRame. Now you are creating different components and added them on 'window' object. As you know a frame is set as visible= false and undecorated=false. So to set a particular JFrame instance you need to call setVisible() method on that instance.

Here you are just creating an instance of GUI and didn't added any component on it. write a statement in constructor as following rather than "window.setContentPane(content);":

setContentPane(content);

and then call setVisible() method on your instance of GUI class i.e d*ogYears.setVisible(true);*!!

Using .setVisible() outside the constructor breaks my GUI

5 Answers5

Linked