Java Singleton + inner class missunderstand

Question

I've been programming Java for the last two months but I'm experienced programmer in python and C. I know I make errors because of this.

I arrive to this question cleaning of warnings my project in Android studio.

I use a Singleton class with inner classes to keep all my configuration parameters in one place and let all other classes access to it with out the need of passing the configuration.

Here is the basic code of my Singleton

public class syscfg {

    public List<CommData> Commlist;
    public static CommConfigIP4 MyCommConfig;// = new CommConfig();

    private static syscfg instance = null;
    private static boolean ConfigStat = false;

    /** JAVA singleton control methods**/
    protected syscfg(){
        // pues eso

        if(ConfigStat == false){
            Log.i("SD_app_log", "SYSCFG: Module Initialization");
            ConfigStat = true;
            MyCommConfig = new CommConfigIP4();
            init_config();
        }else{
            Log.i("SD_app_log", "SYSCFG:  Module Loaded");
        }
    }

    public static syscfg getInstance(){
        if(instance == null){
            instance = new syscfg();
        }
        return instance;
    }

    public class CommConfigIP4{
        public int discoveryPort = 30303;
        public  byte[] MyMAC;
        public  String MyType = "";
        public  String MyIP;

        public  byte[] getIPbytearray(){
//            byte[] IPout= new byte[4];
            try{
                byte[] IPout = (InetAddress.getByName(MyIP)).getAddress();
                return IPout;
            }catch (Exception e){
                return null;
            }

        }

In my communications java file/class I have:

public class Communications {

    private syscfg CFid ;
    ...
    public Communications(Context ctx){
        ...
        CFid = syscfg.getInstance();
        init_comms(); //init_comms calls whoami
    }

    private void whoami (){
        ...
        CFid.MyCommConfig.MyType = netint.getName();
        ...
    }
}

So, when I first had all elements(variables, classes and methods) in syscfg as static android studio showed a warning saying Static member accessed via instance reference. After some research and documentation I found a recommendation not to use static variables and methods and I tried to eliminate them all. But then I get a nullpointexception error in

CFid.MyCommConfig.MyType = netint.getName();

With the debugger I found that CFid.MyCommConfig = null

I use singleton to avoid using static on syscfg class and access via instantiation and not using the class name.

Now my singleton code is like the one posted here with CommConfigIP4 static and I have the warnings again that recommend me using:

syscfg.MyCommConfig.MyType = netint.getName();

instead of using the instance to acces de configuration.

What is happening here? What I'm missing?

Thanks, Guillermo

Singleton's are a bad idea. Google has worked hard to drive them out of their code base. You should, too. https://code.google.com/p/google-singleton-detector/ — duffymo, Sep 23 '15 at 16:44
That class isn't Singleton: as well as the lack of synchronisation in the factory method, a class in any package can extend it, so you can't control who creates it. — Andy Turner, Sep 23 '15 at 16:50
You should try to use Java naming conventions too: ClassesLikeThis, variablesLikeThis, STATIC_CONSTANTS_LIKE_THIS. It is quite hard to follow this code as is. — Andy Turner, Sep 23 '15 at 16:55
*Abusing* and *misusing* the singleton pattern is a bad idea. Google's code base is what(?) billions of LoC? Singletons are not the hammer to all nails but not the spawn of satan either. — ChiefTwoPencils, Sep 23 '15 at 17:45
@duffymo Can you give any evidence to your "singleton's a bad idea" statement? — Alexander Kulyakhtin, Sep 23 '15 at 18:18
@duffymo that project looks like it has been dead for quite a long time. — Andy Turner, Sep 23 '15 at 22:09
In the case I would like to follow google conventions and erase singletons of my project, what would be the best element I can use to keep my configurations and access to read or write from any other class or activity of my android java project? making it static and accessing via class name is the only way? I haven't test it yet but is this thread safe? The nature of my application controlling a big Hardware, big in terms of computation and number of devices, forces me to use many runnable and AsyncTask and other multithread options. — taquionbcn, Sep 24 '15 at 07:48
Also sorry for not using the Java name convention I will do a refactor to solve this "issue" for next posts. — taquionbcn, Sep 24 '15 at 07:49
I found http://stackoverflow.com/questions/1300655/whats-alternative-to-singleton — taquionbcn, Sep 24 '15 at 08:11

score 0 · Accepted Answer · answered Sep 23 '15 at 16:54

In your whoami() method you make this reference:

CFid.MyCommConfig.MyType = netint.getName();

... however, "MyCommConfig" is a static property of the class "syscfg", while the variable "CFid" refers to an instance of this class. In other words, all instances of "syscfg" (as well as the class definition itself) all refer to the same copy of the "MyCommConfig" variable (which is what static means).

For this reason it is less confusing to refer to the "MyCommConfig" variable by saying "syscfg.MyCommConfig" as this makes it clear you are referring to a static variable rather than an instance one.

By the way, you should consider following the standard Java code conventions for capitalising class names and variables as this will make your code more readable for other Java programmers.

score 0 · Answer 2 · answered Sep 23 '15 at 17:01

0

You should declare your nested class to be static:

public static class CommConfigIP4

And then uncomment the code which initializes the static variable at the top of your outer class.

answered Sep 23 '15 at 17:01

Andy Turner

137,514
11
162
243

iperezmel78 · Answer 3 · 2015-09-23T19:00:53.860

To avoid these warnings you should implement a singleton this way. I have changed some code to make it understandable:

public class Syscfg {

    public Config getConfig() {
        return c;
    }

    // Obtain the same instance always
    public static Syscfg getInstance() {
        return s == null ? new Syscfg() : s;
    }

    // Important for singleton
    private Syscfg() {
        c = new Config();
    }

    private static Syscfg s;
    private Config c;

    class Config {

        public String[] getConfigs() {
            return configs;
        }

        private String[] configs = {"10.10.10.10", "userName", "userPass"}; 
    }

}

So, if you want to know the configs used in another class, for instance Test class then you can use

public class Test {

    public static void main(String[] args) {
        System.out.println(Arrays.toString(Syscfg.getInstance().getConfig().getConfigs()));
    }

}

The result is: [10.10.10.10, userName, userPass]

I suggest you to use ENUM singleton approach, due to provide serialization machinery for free and guarantee against multiple instance (reflection attack) as per Josh Block at Effective Java ;) — Rodrigo Gomes, Sep 23 '15 at 17:48

Java Singleton + inner class missunderstand

3 Answers3