Java – thread safe enum singleton

Enumeration helps create singles I know enumeration methods are not thread safe, so I try to make them thread safe Anyone can confirm whether this implementation is correct Can static and volatile be used in so many places and can be optimized? Since the inner class is private, I have to create a function in the enumeration to access the inner class function Can it be optimized?

import java.util.Date;

public enum SingletonWithEnum {
    INSTANCE;   

    private static class Singleton{

        private static volatile int count;
        private static volatile Date date;      

        public static  int getCount() { return count;}

        public static void setCount(int countParam) { synchronized(Singleton.class){ count = countParam; }}

        public static Date getDate() {  return date;}

        public static void setDate(Date dateParam) { synchronized(Singleton.class){ date = dateParam;}}

        public static String printObject() {
            return "Singleton [count=" + getCount() + ",date=" + getDate() + "]";
        }

    }

    public int getCount() { return Singleton.getCount();}

    public void setCount(int countParam)    {Singleton.setCount(countParam);}

    public Date getDate() { return Singleton.getDate();}

    public void setDate(Date dateParam) {Singleton.setDate(dateParam);}

    public String toString(){return Singleton.printObject();}
};

I use it like this

SingletonWithEnum object1 = SingletonWithEnum.INSTANCE;
object1.setCount(5);
object1.setDate(new Date());

Solution

First, you don't need nested classes in your enumeration You only need to define members and methods in the enumeration itself, that is

enum Blah {
  INSTANCE;
  private int someField;
  public int getSomeField() { return someField; }
}

You can now access the singleton method in the following ways:

int someField = Blah.INSTANCE.getSomeField();

In addition, making members static is an anti - pattern here, because a singleton instance should have its members So they should be instance variables, not static variables In fact, only one singleton ensures that there is only one instance of each member in your JVM

In terms of thread safety, I personally prefer atomic variables to volatile, for example:

private final AtomicInteger count = new AtomicInteger();
private final AtomicReference<Date> date = new AtomicReference<>(new Date());

Note that they must be declared final to be truly thread safe, because atomic variables themselves do not change, although their values can

If you only need coded operations, the volatile variable should work Atomic variables provide more operations than their volatile counterparts, such as compareandset for Java 7 and getandupdate and updateandget for Java 8 For discussion, see this

However, if your member variables are thread safe and their thread safety policies are independent, you can declare them (atomic / volatile), and you don't need to worry about the security of the methods in the singleton If you need to update two variables atomically at once, for example, you must reconsider the design and introduce appropriate locks (when setting and obtaining their values)

It is important to modify the date object very carefully The date is not thread safe, so I strongly recommend returning the copy when making changes and replacing the instance with the copy, that is (assuming you are using atomicreference),

public Date getDate() { return new Date(date.get().getTime()); }
public void setDate(Date d) {
  date.set(new Date(d.getTime()));
}

Finally, I strongly recommend Brian Goetz's concurrency in practice and Joshua Bloch's effective java to learn more about concurrency and singleton mode respectively