Eagerestwolf
Eagerestwolf

Reputation: 388

Is it poor practice not to assign an instance of an object to a variable?

Ok, so I have been java programming for a bit now, and occasionally I find that I need to create a new instance of an object, but I don't need the variable for reference, such as working with GUI's. When I work with a GUI, I will do something like this:

public class MainWindow extends JFrame {

    /**
     * Creates new form MainWindow
     */
    public MainWindow() {
        initComponents();
    }

    private void initComponents() {
        /*
         * Setup main properties of window 
         */
        JComponent contentPane = (JComponent)getContentPane();
        contentPane.setBorder(BorderFactory.createEmptyBorder(20, 20, 20, 20));
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setJMenuBar(new MenuBar());
        setTitle("Window Title");
        setMaximumSize(new Dimension(1280, 720));
        setMinimumSize(new Dimension(1280, 720));
        setPreferredSize(new Dimension(1280, 720));
        setSize(1280, 720);
        setLocationRelativeTo(null);
        setResizable(false);

        //Add other stuff here

        setVisible(true);
    }
}

That way, whenever I need to instantiate the GUI, I just call:

new MainWindow();

However, I notice that Netbeans gives me a hint regarding this, so I know that it is not proper practice to do it that way.

That being said, what is the correct way to handle object instantiation when you don't need the variable?

Upvotes: 1

Views: 245

Answers (3)

kai
kai

Reputation: 119

Your object is not referenced by any other object and could potentially be garbage-collected. If you only need a single instance in your application, you could use static method in the same class to implement a singleton, like this (the code below has not been compiled):

public class MainWindow extends JFrame {

    private static MainWindow instance;

    public static MainWindow getMainWindow() {
        if (instance == null) {
            instance = new MainWindow();
        }
        return instance;
    }

    /**
     * Creates new form MainWindow
     */
    private MainWindow() {
        initComponents();
    }

    private void initComponents() {
        // ... skip original code for brevity ...
    }
}

Note that you could make the constructor private in this case.

EDIT corrected the instance to null comparison as per comments.

Upvotes: 3

Jesse Webb
Jesse Webb

Reputation: 45243

I don't think it is bad practice; I agree with Sotirios Delimanolis that "It's worse practice if you assign it to a variable that you don't use."

But I do think that you can do a couple of things to make it clearer and easier to maintain.

  • Wrap the call to new ... in a method that describes what it does. This makes it clear in code what your intent of calling the constructor is. It also makes it easy to change the way to do it later, if you need to.

Example:

private void instantiateTheGui() {
    new MainWindow();
}
  • I think it might be a bad idea to do all that work in the constructor. Does every usage of that object want the same properties? Size? Visibility? Etc.? I would argue that the last line (setVisible(true);), at least, should be invoced by whatever code is calling the constructor because it may not want it to be visible immediately.

Now your code suddenly becomes:

private void instantiateAndShowTheGui() {
    MainWindow window = new MainWindow();
    window.setVisible(true);
}

Now we have a variable, and a legitimate usage of it. I wasn't trying to get a variable, it happened with good design and responsibility separation.

My point is that there isn't anything inherently wrong about calling an object constructor without storing it in a variable, but I have never come across (IMO) nicely designed code that uses this pattern. It is not bad, but clean code often doesn't run into the problem.

Upvotes: 2

Christian Kuetbach
Christian Kuetbach

Reputation: 16060

It is bad practice to create a class like this, because:

If onother developer reads this code (or your future-you) you may not remember why there is an unused object like that.

I would rewrite is a litle like and write something like

new MainWindow().show();

show() will call the initComponent().

Upvotes: 1

Related Questions