patternjavaMinor
Simple Java Singleton
Viewed 0 times
singletonsimplejava
Problem
Below is a Singleton that I designed as a piece of sample code for a website I'm working on for fun. The goal is to use the Singleton to hold an
Singleton.java
```
package design.patterns;
/**
* This class is a Singleton, we can only have 1 instance of it no matter what!
*
* We mark the class final so that no other classes try to extend it. Some
* designs would want the Singleton to be extendable but we do not!
*/
public final class Earth {
// A static instance of an Earth object
private static Earth earth;
// Some earthly variables
private long ageInYears = 4500000000L;
private float daysForFullOrbit = 365.26f;
private float degreesOfAxisTilt = 23.4f;
private long population = 7046000000L;
// Prevent client from instantiating new Earth objects
private Earth() {
}
/**
* Global access point (no pun intended :)
*
* Synchronized so its thread safe.
*/
public static synchronized Earth getInstance() {
// "Lazy load" an Earth Object
if (earth == null) {
earth = new Earth();
}
return earth;
}
// Basic getters and setters
public double getAgeInYears() {
return ageInYears;
}
public void setAgeInYears(long ageInYears) {
this.ageInYears = ageInYears;
}
public float getdaysForFullOrbit() {
return daysForFullOrbit;
}
public void setdaysForFullOrbit(float daysForFullRotation) {
this.daysForFullOrbit = daysForFullRotation;
}
public float getDegreesOfAxisTilt() {
return degreesOfAxisTilt;
}
public void setDegreesOfAxisTilt(float degreesOfAxisTilt) {
this.degreesOfAxisTilt = degreesOfAxisTilt;
}
public double getPopulation() {
return population;
}
public void setPopulation(int population) {
th
Earth object since Earth is unique and there is only 1 of them. Please let me know your thoughts on the implementation. Singleton.java
```
package design.patterns;
/**
* This class is a Singleton, we can only have 1 instance of it no matter what!
*
* We mark the class final so that no other classes try to extend it. Some
* designs would want the Singleton to be extendable but we do not!
*/
public final class Earth {
// A static instance of an Earth object
private static Earth earth;
// Some earthly variables
private long ageInYears = 4500000000L;
private float daysForFullOrbit = 365.26f;
private float degreesOfAxisTilt = 23.4f;
private long population = 7046000000L;
// Prevent client from instantiating new Earth objects
private Earth() {
}
/**
* Global access point (no pun intended :)
*
* Synchronized so its thread safe.
*/
public static synchronized Earth getInstance() {
// "Lazy load" an Earth Object
if (earth == null) {
earth = new Earth();
}
return earth;
}
// Basic getters and setters
public double getAgeInYears() {
return ageInYears;
}
public void setAgeInYears(long ageInYears) {
this.ageInYears = ageInYears;
}
public float getdaysForFullOrbit() {
return daysForFullOrbit;
}
public void setdaysForFullOrbit(float daysForFullRotation) {
this.daysForFullOrbit = daysForFullRotation;
}
public float getDegreesOfAxisTilt() {
return degreesOfAxisTilt;
}
public void setDegreesOfAxisTilt(float degreesOfAxisTilt) {
this.degreesOfAxisTilt = degreesOfAxisTilt;
}
public double getPopulation() {
return population;
}
public void setPopulation(int population) {
th
Solution
The code you have is nicely formatted, and well documented, etc. ... but, as a singleton, it has a number of problems....
The two most glaring are:
-
It is not a singleton
-
The synchronization.
You suggest in your comments that the
As an example of a singleton 'best use case', this one has some problems... ;-)
But, that's sort of OK, since Earth has problems anyway!
The two most glaring are:
-
It is not a singleton
Earth real = Earth.getInstance();
Earth.setEarth(null);
Earth alternate = Earth.getInstance();
if (real != alternate) {
System.out.println("Oops...");
}-
The synchronization.
You suggest in your comments that the
getInstance() needs to be synchronized to avoid thread problems... but your other setter/getter methods are not synchronized.... as a result, threads all over the place can be getting stale, wrong, and otherwise incomplete populations, ages, etc.As an example of a singleton 'best use case', this one has some problems... ;-)
But, that's sort of OK, since Earth has problems anyway!
Code Snippets
Earth real = Earth.getInstance();
Earth.setEarth(null);
Earth alternate = Earth.getInstance();
if (real != alternate) {
System.out.println("Oops...");
}Context
StackExchange Code Review Q#39710, answer score: 5
Revisions (0)
No revisions yet.