Basic structure for interactive single window application

This is the basic structure I usually end up with whenever I'm writing a simple application. More recently I've learned about singletons and have started to incorporate them into my design, but other that that I haven't changed much about it in recent times.

I've been wondering if there is anything in here that I could improve on or that is considered bad practice.

Main

import javax.swing.*;

public class Main {
    public static void main(String[] args) {
        try {
            UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
        } catch (ClassNotFoundException | InstantiationException | UnsupportedLookAndFeelException | IllegalAccessException e) {
            e.printStackTrace();
        }

        Controller.getInstance();
    }
}

Controller

import javax.swing.*;

public class Controller {
    private static Controller instance;

    private static final int FPS = 30;
    public static final int WIDTH = 600;
    public static final int HEIGHT = 800;

    private final JFrame frame;

    private Timer timer;

    public static Controller getInstance() {
        return instance != null ? instance : (instance = new Controller());
    }

    private Controller() {
        //Do setup
        frame = new AppFrame();

        timer = new Timer(1000/FPS, e -> update());
        timer.start();
    }

    private void update() {
        //Update model
        frame.repaint();
    }
}

AppFrame

import javax.swing.*;

public class AppFrame extends JFrame {
    public AppFrame() {
        super("My Example App");
        setResizable(false);
        setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);

        add(new AppPanel());

        //Do more setup if applicable

        pack();
        setLocationRelativeTo(null);
        setVisible(true);
    }
}

AppPanel

```
import javax.swing.*;
import java.awt.*;

public class AppPanel extends JPanel {
private final Dimension DIMENSION = new Dimension(Controller.W

The code looks decent enough. I'd like to point out three issues, though, all related to timing/multi-threading.

GUI → EDT

All GUI operations should happen on the Event Dispatch Thread. Java boots into the Main thread, which is different from the EDT. Your Controller.getInstance() should be called on the EDT:

EventQueue.invokeLater(Controller::getInstance)

Controller.instance should be guarded

Currently, the instance if filled in via the Main thread. If the GUI (on the EDT or through the timer) later requests the instance, there is no guarantee they will see it, or they might see it in an inconsistent/incomplete state.

This can be solved by either making instance volatile or, perhaps better, making getInstance() a synchronized method.

repaint() is on a best-effort basis

It looks like this is the basis for a game or other graphically heavy application (judging from FPS). Swing (and its timers) may not give you the timing guarantees you'd need for things like game logic.

Consider using a game loop with a Canvas and a BufferStrategy, disabling redraw requests, and taking over from there.

Isn't the Timer good enough? I'm tempted to say, yes, it might be good enough, if the game logic is simple and not intense. Though you're no longer blocking the EDT with Thread.sleep (big win), the actions that the timer calls are still executed on the EDT. This makes handling GUI elements and fields much simpler (no sync needed), but heavier stuff may result in jitter. Worse: heavy stuff on the EDT may make your game loop jitter.

In short: the timer is simpler and probably good enough. The game loop in a separate thread (say, your main thread) is more heavyweight but also more reliable and more likely to benefit from hardware acceleration.

Nitpicking

• Controller.getInstance seems like an odd method to call from main to kick things off. Maybe make it Controller.run() or Controller.getInstance().run() or some such that would better imply the side effect of running your program.

-
getPreferredSize returns a mutable final variable (Dimension is mutable). Perhaps you could try this:

public Dimension getPreferredSize() {
    return isPreferredSizeSet() ? super.getPreferredSize() : new Dimension(DIMENSION);
}

StackExchange Code Review Q#123036, answer score: 3