Singleton design for small/medium sized music player project

Question

I am refactoring the code for a music player I wrote in Java/JavaFX.

I have a few key objects which are accessed throughout the program by various other classes. I think there will be four or five by the time I finish moving everything around. There is only ever one instance of these objects instantiated for the life of the program.

Is this approach to making these key Objects easily available when needed good architecture? Is there a better approach to this issue?

public class Hypnos extends Application {

    private static AudioPlayer player;
    private static FXUI gui;
    private static Library library;
    private static Queue queue;

    ... 

    public static AudioPlayer getPlayer() { 
        if ( audioPlayer == null ) {
            throw new IllegalStateException();
        }

        return player;
    }

    public static FXUI getGUI() {
        if ( ui == null ) {
            throw new IllegalStateException();
        }
        return gui;
    }

    //etc 

    @Override
    public void start ( Stage stage ) {
        ...
        library = new Library();
        queue = new Queue();
        player = new AudioPlayer();
        gui = new FXUI ( stage );
        ...
    }

    public static void main ( String[] args ) {
        launch( args ); //This calls start() above. 
    }
}

Now, when I need them, I can just call Hypnos.getPlayer(), or Hypnos.getGUI(), without having to pass a bunch of objects around through the various classes and methods.

Answer 1

Is this approach to making these key Objects easily available when needed good architecture? Is there a better approach to this issue?

The much maligned singleton pattern occasionally has it's uses but I'm not seeing it here. You say you have four or five key objects that are needed by "various other classes". Let me show you an alternative that is more flexible and respects the "only ever one instance" requirement.

public static void main(String[] args) {
    Stage stage = launch(args);

    Library library = new Library();
    Queue queue = new Queue();
    AudioPlayer player = new AudioPlayer();
    FXUI gui = new FXUI(stage);

    Resources keyResources = new KeyResources(library, queue, player, gui);

    VariousOtherClass1 variousOtherClass1 = new VariousOtherClass1(keyResources);
    VariousOtherClass2 variousOtherClass2 = new VariousOtherClass2(keyResources);
    VariousOtherClass3 variousOtherClass3 = new VariousOtherClass1(
        keyResources, 
        variousOtherClass1, 
        variousOtherClass2);

    variousOtherClass3.start();
}

Build it only in main, once, and it will only ever have one instance. This is good old fashioned reference passing. The fancy new term for it is Dependency Injection. The pattern is simple, construct what you need, connect them together, and start one of them ticking. This keeps behavior from mixing with construction.

KeyResources (a name that could stand improvement) is a parameter object. Done this way all of these resources can be replaced without having to rewrite KeyResources.

The main advantage here is that the VariousOtherClasses (a name desperately in need of improvement) don't KNOW where to find their resources. They get told what their resources are. This means they don't dictate that you have a global resource. That different resources could be given to them without requiring a rewrite. That if some suddenly need different resources the others don't break because you changed the only place they get defined. And it makes their dependence on these resources explicit. Oh, and sure, testing.

Answer 2

Is this approach to making these key Objects easily available when needed good architecture? Is there a better approach to this issue?

Easy availability for objects that are needed in a context does not affect the subject architecture. Objects that are needed should ALWAYS be easy available.

The more interesting question that most of the developers do not think about: Is it really neccessary to make certain objects easy accessable?

You can have two sides. Each of them maybe neccessary or not. You have to decide proper. So there are 4 possible results:

1. Easy access -> neccessary
2. Easy access -> unnecessary
3. Difficult access -> neccessary
4. Difficult access -> unneccessary

While 1. obviously makes sense unneccessary easy access (2.) will increase the number of dependencies to manage within a system. That is with wild static access to singletons can do harm to your architecture.

The more difficult decisions are when it seems to be a problem to access a special object. You have to be very careful as to break isolation and introduce new dependencies is a very important decision. If you break isolation all the time because you want to have access to an object then this will cause harm to your architecture as well.

So "difficult access to objects" is your friend. It is an indicator for architectural decisions to make.

To your code:

public static AudioPlayer getPlayer() { 
    if ( audioPlayer == null ) {
        throw new IllegalStateException();
    }

    return player;
}

Here you have a hidden state pattern as you throw an exception if the static var is not initialized by the start method. Further more you mixed class scope with object scope in a problematic way.

It is a poor design to depend on time when it comes to var initialization. Here a pure Singleton-Pattern is far better to ALWAYS have a proper initialized variable/object. Further more you abstract from the creation process of the object. But I only take these two possibilities into account as there are a lot more and better approaches. But you can go with this first:

public class AudioPlayer {

    AudioPlayer () {
    }

    public static AudioPlayer getPlayer() { 
        if ( audioPlayer == null ) {
            player = new Audioplayer();
        }

        return player;
    }

}

[...] without having to pass and cache a bunch of objects around through the various classes and methods. [...]

This is the main reason I use singletons for objects that have a global character and any other scope does not make sense. A good indicator for a singleton-object is exactly: If you have to pass the object to any class and method in your system. If security is important to your application the most obvious object to be globally accessable is the current logged in User. So there it does not make sense to pass this object around through the whole application as this will lead to a lot of boiler plate code and inexpressive method signatures.

After all you have to consider how large your application is. The larger your application is the more important architectural decisions become. If you are developing this application on your own you will face architectural problem later than when you are developing in a team.

I do not want to be rude but I do not think that a music player developed by one person matches a size where architectural decisions get relevant. I suggest to you some more basic code quality measures. Maybe you post some code on "Stack Overflow - Code Review" to get feedback on code quality.