Testing a class with private constructor

Question

Im trying to come up with the best way to test a cache class that im currently using.... i would like to substitute ClientFactory below when this class is run in tests.... I like to leave the structure of the class as much as possible but since it has a private constructor im struggling to think of the best way to test it..

public class MyCache {

private final long TIME_OUT

private static MyCache instance = null;

private final HashMap<String, MyObject> cache = new HashMap<String, MyObject>();

private MyCache() {
}

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

public MyObject getDetails(String id) throws Exception {

    MyObject myObject = cache.get(id);
    if (myObject != null) {
        return myObject;
    } else {
        try {

            // want to be able to replace ClientFactory with test stub
            Client client = ClientFactory.createClient();
            myObject = client.getMyObject(id);

        } catch (NotFoundException nf) {
           .... log error
        } 

        return myObject;
    }
}

}

Even with a public constructor, since createClient is a static method, it's very hard to mock it. Learn about dependency injection, don't use the singleton anti-pattern, inject a ClientFactory in the public constructor, make sure createClient is an instance method and not a static method. — JB Nizet, Apr 16 '16 at 12:33
I would also consider not reinventing the wheel. The cache isn't thread-safe (which might or might not be a problem), and there are ready-to-use caches available, in Guava, for example. — JB Nizet, Apr 16 '16 at 12:34
hey just coming back to this again, when you say 'inject a ClientFactory in the public constructor' .. there is no public constructor... do you mean pass it to the getInstance? — blu10, Apr 22 '16 at 13:30
No. I mean that there should be a public constructor, and you should simply rely on the dependency injection framework to create a single instance of this class. — JB Nizet, Apr 23 '16 at 06:22
i want to only have one instance only though, having a public constructor hints that its no a singleton no? — blu10, Apr 25 '16 at 08:49
btw, there is no dependency injection framework on the project im working on as yet — blu10, Apr 25 '16 at 08:57

score 1 · Accepted Answer · answered Apr 16 '16 at 14:09

You can do a lot of things, but I think that from testing perspective Singleton-pattern is not a good choice.

If you are using Mockito, you should extract the ClientFactory.createClient() call into a package-public (default) function. Mockito can spy singletons: spy(MyCache.class), and you can change the behavior of the extracted function. So you replaced the ClientFactory.
You can replace your private constructor with a package-public contructor, and also you need to extract function mentioned in the first solution. After these changes you can extend MyCache in the test class (without Mockito).
You can extract the functionality of MyCache into a package-public class, which is not singleton (but can't call from outside). You can test it nicely, and MyCache will be only a singleton wrapper of the extracted class.

I think reflection is anti-pattern, but I know that the default access modifier (the empty string) is also ugly a little bit.

A few words about your singleton-pattern. This is not bad, if you have only one thread, but if you are in a multi-threaded environment, you need this codes:

// you need volatile, because of JVM thread caching
private static volatile MyCache instance;

private MyCache() {}

public static MyCache getInstance() {
    if (instance == null) {
        synchronize(MyCache.class) {
            // yes, you need double check, because of threads
            if (instance == null) {
                instance = new MyCache();
            }
        }
    }
    return instance;
}

I like the sound of option three but not quite sure... how id structure it... any pseudocode you could share — blu10, Apr 18 '16 at 08:03
Your MyCache file will be the same, but constructor will be package-public and no getInstance function, and there will be a MyCacheWrapper class, which getDetails function calls its instance (MyCache) getDetails function, and getInstance function creates a new MyCache(). Is it ok or should i write the two files? — , Apr 18 '16 at 11:08
ok i think i get you.... i was also thinking or adding a setter method to this class ... might be simpler...what do you think? — blu10, Apr 18 '16 at 13:44

Testing a class with private constructor

1 Answers1