Java Singleton getter/setter

Question

I've created working setters/getters for an application, that will use a few different classes. This classes will use one class, that will store all data. I know, when I will use a standard constructor of container class, I'll get tons on nulls, due to different instance of container class.

I've created container class within a singleton, that works, but I wanted to ask if anything could be done better, or if this code follow the best practice.

public class Container {

    private String appName = null;
    private String appOwner = null;

    private Container() {
        System.out.println("Start");
    }

    private static class SingletonHolder {
        private final static Container INSTANCE = new Container();
    }

    public static Container getInstance() {
        return SingletonHolder.INSTANCE;
    }

    public String getAppName() {
        return appName;
    }

    public void setAppName(String appName) {
        this.appName = appName;
    }

    public String getAppOwner() {
        return appOwner;
    }

    public void setAppOwner(String appOwner) {
        this.appOwner = appOwner;
    }
}

Sample class, that will use this container:

public class SecondClass {
     Container ctn = Container.getInstance();
}

Right now, when I use in main class:

ctn.setAppOwner(owner);

I get a proper value in any other classes, when I call this:

Container ctn = Container.getInstance();
ctn.getAppOwner();

Is that a good approach?

Answer 1

There are a couple bad things in the code

First of all, this is weird in many ways:

private Container() {
    System.out.println("Start");
}

You print out "Start" to the standard output. This basically restrict any usage of this class to have access to a console. What if it is used in a GUI application? What if this is used in a server application?

Also, "Start" what? Whoever read the console will have no clue what is started?

The object is constructed as a static field, and Java did not guarantee when these static constructor are run (they only guarantee it is run before the class methods is run, so it is basically you have a console print that is shown in non-deterministic time.

This is not great too.

public void setAppName(String appName) {
    this.appName = appName;
}

Whoever got access to the Container object instance can call this method, which means you basically have a global variable that anyone can access. That's a bad idea, because it will makes debugging really difficult.

From these classes, it appears to me that all you needed is to let the app name and app owner initialized once and get access anywhere, you can make that by simply having a Configuration class with a static property.

public class Configuration
{
    private static String appName;
    private static String appOwner;
    public static void Initialize(String appName, String appOwner)
    {
        Configuration.appName = appName;
        Configuration.appOwner = appOwner;
    }
    public static String AppName() { return Configuration.appName; }
    public static String AppOwner() { return Configuration.appOwner; }
}

That would be it, simple and clear.

Answer 2

The technique is pretty standard; an alternative:

public enum Container {
    INSTANCE;
    ...
}

However setters can be called more than once, and even concurrently; so that is an area still to be tackled.

The chance of using immutable objects with a once-only initialisation, and possibly only having a

public static final String APP_NAME;

will be given away.

The reason is, the concentration on the data model - of say a Model-View-Controller paradigm.

My proposal: You could have a singleton Controller, the Application(Controller) or such. This main controller could hold the model, more than one container. Access would by MVC go via the controller.

This allows initialisation of some container with values, and specific work oriented access to the data model. (Several data, states.)

This will be much more satisfying for a developer, as now you start with many getters and setters, and with a controller layer, much boiler plate code falls away. Ideally only business logic operations remain.

Answer 3

First off I would like to point out that "setters" are usually called mutators and "getters" are called accessors; however that is not usually important in practice(In my experience so far).

As already mentioned, the following command is pretty obsolete and is not required.

System.out.println("Start");

Also a quicker way to initialize an object of this class may be to use an override constructor below the first constructor as such:

private String appName;
private String appOwner;

public Container() {
    appName = null;
    appOwner = null;
}

public Container(int appName, int appOwner)
{
    this.appName = appName;
    this.appOwner = appOwner;
}

This will save you the time of having to call 2 mutator or setter methods every time you create a new instance of the object.