CODEWITHSUNDEEP
javastaticsingleton
CoolBeans
CoolBeans

Reputation: 20800

Singleton with initializing a static member

In the code snippet below when I originally designed it, the "next number" needed to send the next incremented value throughout the execution of the application. So I made the class a singleton. However, with some recent change in requirements I needed to do a reset on the "next number". I just added a reset method to do that. However, it definitely violates the Singleton pattern and also I know it is not a good idea to initialize a static member this way.

What do you think I should do instead? 

public final class GetNextNumber {
    private static GetNextNumber instance; 
    private static Integer nextNumber=1;
    private GetNextNumber() {
    }
    public static synchronized GetNextNumber getInstance() {
        if(instance==null){
            instance = new GetNextNumber();
        }
        return instance;
    } 
    protected Integer getNextNumber(){
        return nextNumber++;
    }
    protected synchronized void reset(){
        nextNumber=1;
    }
    public Object clone() throws CloneNotSupportedException {
        throw new CloneNotSupportedException();
    }
}

Upvotes: 0

Views: 1465

Answers (2)

Richard Fearn
Richard Fearn

Reputation: 25491

Looks OK to me - except for two things:

  • getNextNumber is not synchronized.
  • since getNextNumber and reset are not static, nextNumber doesn't need to be static, either.

You could use an AtomicInteger to avoid having to make your getNextNumber and reset methods synchronized:

public final class GetNextNumber {

    private static GetNextNumber instance;

    private AtomicInteger nextNumber = new AtomicInteger(1);

    private GetNextNumber() {
    }

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

    protected Integer getNextNumber(){
        return nextNumber.getAndIncrement();
    }

    protected void reset(){
        nextNumber.set(1);
    }
}

For futher discussion on this, see for example The Atomic classes in Java 5: AtomicInteger and AtomicLong:

Before Java 5, we had to write classes with access to the counter variable in synchronized blocks or methods, or else use a volatile variable which is a lighter form of synchronization but with the risk that some updates could be missed if they happen concurrently. An AtomicInteger can be used as a drop-in replacement that provides the best of both worlds...

Upvotes: 2

MeBigFatGuy
MeBigFatGuy

Reputation: 28568

why aren't the fields just instance variables? theres no need for static here.

reset doesn't need to be synchronized either, unless getNextNumber is as well.

Upvotes: 2

Related Questions