Singletons

PhillS

I work mainly on J2EE software (the primary product at the company I work for is a document management system). I joined in 2004, but a lot of the code dates back quite a way to 2001 or before, and as such a lot of the code is a bit "WTF-esque" (such as a lot of the 'controller' layer logic being in JSP pages, passing ResultSet objects freely around etc... I guess some of it is down to the MVC architecture not really taking hold until after the initial development was done, but still it's no excuse!).

But some of the code is just pretty bad. There was one class which was about 1,700 lines long... I managed to cut it down to under 700 just by removing the duplicate "cut-and-paste" code. (I'd post it up here, but the WTF is the whole thing, rather than an individual bit).

There is some stuff which is just plain stupid, though. This is an example of someone trying to create a singleton (I've anonymized it a bit):

public class propertyreader extends SomePropertyReader
{
	private static propertyreader instance;

	public propertyreader() throws IOException
	{
		this.getInstance();
	}

	private static propertyreader getInstance() throws IOException
	{
		if (instance == null)
		{
			instance = new propertyreader("some.properties");
		}
		return instance;
	}

	private propertyreader(String fileName) throws IOException
	{
		super();
		this.getProperties();
		this.setInternals();
	}
 
	(etc...)
}

First post, so hoping that the forum software won't screw up the HTML...

Joost_

Subtle.

firewireguy

You now I've never seen a thread safe singleton implementation that everyone agreed was "perfect".

triso

@firewireguy said:

You now I've never seen a thread safe singleton implementation that everyone agreed was "perfect".

<font size="5">H</font>ow
about one with a synchronized public constructor? Maybe an
overridden clone() method too. Don't forget about subclassing and
getClass() workarounds as well...The list goes on.

brazzy

@triso said:

@firewireguy said:
You now I've never seen a
thread safe singleton implementation that everyone agreed was "perfect".
<font size="5">H</font>ow
about one with a synchronized public constructor? Maybe an
overridden clone() method too. Don't forget about subclassing and
getClass() workarounds as well...The list goes on.

First, if a Java class has a public constructor that can complete
successfully more than once, it is not a singleton, since a constructor
always creates a new object.

Second... it's not that difficult. Perfect threadsafe Singleton in Java:

public final class Singleton

{

      private static Singleton instance = new Singleton();

      private Singleton(){}

      public static Singleton getInstance()

      {

             return instance;

      }

}

Final so it can't be subclassed, instance is created during class
initialization (still lazy since we have late binding), thus
synchronized by the JVM. Object.clone() will throw an exception. Not
serializable, of course.

Admittedly, you can still corcumvent all this by using reflection
(AccessibleObject.setAccessible()), but that requires more expertise
than your typical incompetent coder has and such needs to be feared
only when running code that might be actively malicious, in which case
duplicated singletons are the least of your worries. But it can be
prevented as well by one entry in the policy file.