3

I have been getting some strange errors with Mongodb, and in Mongodb, you are supposed to mainatin the Mongo singleton. I just wanted to make sure that this is infact valid.

public class DBManager {
    public static Mongo mongoSingleton = null;

    public static synchronized void getMongo(){
         if(mongoSingleton == null){
              mongoSingleton = new Mongo();
         }
         return mongoSingleton;
    }
}

Thanks!

wfbarksdale
  • 7,498
  • 15
  • 65
  • 88
  • 1
    Code does not compile - getMongo() returns void. That said, it is a valid singleton pattern - but slow - look up double checked locking. And singleton field should be private. – ptyx May 23 '12 at 23:28
  • 5
    I'll save you [a Google trip](http://en.wikipedia.org/wiki/Double-checked_locking) – kentcdodds May 23 '12 at 23:30
  • 1
    @ptyx but remember that the variable needs to be volatile for that to work correctly – ratchet freak May 23 '12 at 23:37
  • If you only want one instance of the object, *just create one instance of it*. Needing this sort of pattern suggests that other code is calling static methods on DBManager rather than passing around an instance of the DBManager. – matt b May 23 '12 at 23:43

9 Answers9

6

You have to set your public member mongoSingleton as private and to hide the default constructor

so 

private static Mongo mongoSingleton = null;

private Mongo() {

}

the class Mongo implementation

public class Mongo {
    private static volatile Mongo instance;
    private Mongo() {
        ...
    }

    public static Mongo getInstance() {
        if (instance == null) {
            synchronized (Mongo.class) {
                if (instance == null) { // yes double check
                    instance = new Mongo();
                }
            }
        }

        return instance;
    }
}

usage

Mongo.getInstance();
pbaris
  • 4,525
  • 5
  • 37
  • 61
  • This is the only true way to ensure that only one instance of the variable exists per runtime. – LINEMAN78 May 23 '12 at 23:49
  • 2
    @LINEMAN78: Well, this is a classical textbook singleton, but it isn't definitely the only approach to implement it and actually also not the best one. – Natix May 23 '12 at 23:59
  • You can also create a factory class so that getInstance doesn't have to be synchronized in order to be thread safe but is still lazily instantiated: public final class Mongo { private Mongo(){} private final MongoFactory { private static final Mongo INSTANCE = new Mongo(); } public static Mongo getInstance() { return MongoFactory.INSTANCE; } } – LINEMAN78 May 24 '12 at 00:04
  • @Natix agreed, there are plenty of arguments suggesting that the enum pattern is best, however it makes the assumption that Mongo doesn't extend anything. I should have said the only answer suggested so far that truely assures a single instance as the above enum example was not for the class Mongo. – LINEMAN78 May 24 '12 at 00:07
5

This is the typical Singleton pattern, but the preferred method in Java is to create an Enum:

public enum DBManager {
    INSTANCE;

    // implementation here
}

You can then refer to the instance via:

DBManager.INSTANCE

Note that in either case (enum or singleton pattern), the result is one instance per ClassLoader, NOT per JVM.

MarkP
  • 51
  • 1
  • This is interesting, I've never seen this before. And IIRC this works only for stateless classes, as enums can't have fields, right? – Jochen May 23 '12 at 23:57
  • Sure, enums can have fields. For example: public enum DBManager { INSTANCE(new ManagableEntity()); private final ManagableEntity entity; private DBManager(final ManagableEntity entity) { this.entity = entity; } public ManagableEntity getEntity() { return entity; } } – MarkP May 26 '12 at 23:25
  • Indeed. I didn't know that, and somehow I wish I still didn't know. There doesn't seem to be any sensible reason for this kind of construct, as it's only syntactic sugar. Very confusing when you think of enums in other languages. – Jochen May 29 '12 at 21:46
  • +1 This should be the prefered way. Also see [this](http://stackoverflow.com/a/71399/869488) – rajesh Mar 14 '13 at 06:31
1

no, because public access to the static member you can change it to whatever instance you want.

More sane would be the following

public class DBManager {
    private static Mongo mongoSingleton = new Mongo();

    public static Mongo getMongo(){
       return mongoSingleton;
    }
}
Bhesh Gurung
  • 50,430
  • 22
  • 93
  • 142
nsfyn55
  • 14,875
  • 8
  • 50
  • 77
1
 public static Mongo mongoSingleton = null;

needs to be 

 private static Mongo mongoSingleton = null;

Other than that, looks good.

dfb
  • 13,133
  • 2
  • 31
  • 52
1

The cheapest way to do this, if you are willing to forego Lazy initialization, is to simply create the Singleton on object creation.

public final class DBManager {
    private static final Mongo mongoSingleton = new Mongo();

    private DBManager() {}

    public static Mongo getMongo() {
         return mongoSingleton;
    }
}

This way, you can avoid the unnecessary synchronization overhead which would otherwise be present on every call to this method. As long as Mongo itself is thread safe, your getMongo() method is thread-safe as well.

Read the Sun developer article on Singletons here.

Jeshurun
  • 22,940
  • 6
  • 79
  • 92
  • 1
    It's not even foregoing lazy initialization, unless you have static methods on the class which you want to invoke before initializing the singleton (or are loading the class with reflection for some reason). Otherwise, the class will only get loaded the first time you access the singleton, which is as lazy as you need in pretty much all cases. – yshavit May 23 '12 at 23:39
1

Just use private static final Mongo mongo = new Mongo(), or even public static final Mongo mongo = new Mongo(). It's simpler and potentially faster.

Enno Shioji
  • 26,542
  • 13
  • 70
  • 109
  • One minor side effect is that this is called whenever the class is loaded, so if you have anything that loads DBManager.class without wanting to initialize the singleton it could be expensive. – LINEMAN78 May 23 '12 at 23:45
1

Lazy initialization of singletons is overrated, usually not necessary and causes more problems than good. The naive approach to make the whole static getter synchronized is bad, because the synchronization (which is a time consuming operation) is performed every time.

There are some better approaches such as double-checked locking or the initialization-on-demand holder, but the simplest and usually the best approach is Josh Bloch's enum approach. Make an enum with a single element and you get a bullet proof singleton.

public enum Mongo {

    INSTANCE;

    // instance fields, methods etc. as in any other class
}

This is almost functionally equivalent to a class with a private constructor and a public static final instance, but with more concise syntax and some nice under-the-hood features that unshakeably guarantee that no other instance can be created, not even through reflection or deserialization.

It is worth noting that this approach isn't necessarily eager. The single INSTANCE is created just after the Mongo class is loaded into the JVM, which doesn't happed right after the program starts - actually it happens on the first time the class is referenced - which in this case means on the first time a static field or a method is accessed.

And if there are no other static fields other that INSTANCE and no static methods, then this really happens after the first time the INSTANCE is accessed:

Mongo mongo = Mongo.INSTANCE;

In other words, it actually is lazy without the problems of explicit synchronization.

Natix
  • 14,017
  • 7
  • 54
  • 69
0

Yes but you don't want your member variable to be public. Users have to come via your synchronised getter

Brad
  • 15,186
  • 11
  • 60
  • 74
0

If you want to see in more details, step by step- you can follow this link- http://www.javabeginner.com/learn-java/java-singleton-design-pattern

Below is one example how to do it properly.

public class DBManager {
private static Mongo mongoSingleton = null;

public static synchronized void getMongo(){
     if(mongoSingleton == null){
          mongoSingleton = new Mongo();
     }
     return mongoSingleton;
}

}

arsenal
  • 23,366
  • 85
  • 225
  • 331