views:

794

answers:

6

Hi, all.

My spaghetti monster consumes XML from several different SOAP services, and the URL for each service is hardcoded into the application. I'm in the process of undoing this hardcoding, and storing the URLs in a properties file.

In terms of reading the properties file, I'd like to encompass that logic in a Singleton that can be referenced as needed.

Change this:
accountLookupURL ="http://prodServer:8080/accountLookupService";

To this:
accountLookupURL =urlLister.getURL("accountLookup");

The Singleton would be contained within the urlLister.

I've tended to shy away from the Singleton pattern, only because I've not had to use it, previously. Am I on the right track, here?

Thanks!
IVR Avenger

A: 

To restate the obvious, Singleton is to be used when all client code should talk to a single instance of the class. So, use a Singleton IFF you are certain that you would not want to load multiple properties files at once. Personally, I would want to be able to have that functionality (loading multiple properties files).

danben
Using a singleton does not preclude loading multiple properties files at once... I have some code that uses a singleton to centralize access to multiple properties files (actually ResourceBundles in my case). Perhaps you meant multiple properties files that contain definitions for the same property names?
Nate
Right, I wouldn't expect that I would have to worry about name clashes if I loaded multiple properties files, since the system shouldn't (necessarily) assume they have anything to do with each other.
danben
Also, even if there aren't name clashes, you may want to have a logical separation of sets of properties (ie accessing sets from different instances). You could namespace them but I wouldn't be crazy about having to do that.
danben
A: 

Singletons are mutable statics and therefore evil. (Assuming a reasonably useful definition of "singleton".

Any code that uses the static (a transitive relationship), is has assumptions about pretty much everything else (in this case, a web server and the internet). Mutable statics are bad design, and bad design makes many aspects go rotten (dependency, understandability, testing, security, etc).

As an example, the only thing stopping late versions of JUnit 3 being used in a sandbox was loading a configuration file in one static initialiser. If it had used Parameterisation from Above, there would have been no issue.

Tom Hawtin - tackline
-1 circular explanation without addressing the question or providing any info
danben
+3  A: 

You haven't said why you need only one of whatever it is which will be getting the URL. If that just involves reading a properties file, I don't think you do need only one. Seems to me that having two threads read the same properties file at the same time isn't a problem at all.

Unless you were thinking of having some object which only reads the properties file once and then caches the contents for future use. But this is a web application, right? So the way to deal with that is to read in the properties when the application starts up, and store them in the application context. There's only one application context, so there's your "only one" object.

Paul Clapham
I'm not sure that I'm going to be able to do this, as a good portion of this application is abstracted into a JAR file whose source is not available to us. What would this entail?
IVR Avenger
IVR Avenger, how do you pass the properties into the Jar file now then?
Poindexter
I would imagine that properties are read exactly as you describe, above. However, that reading happens outside of a realm that I have access to. My code extends classes whose source I don't have, and those classes are reading specific properties. Ideally, I'd change the base class so that these URLs are available just like existing properties, but I don't have access to the base class. Yet... :-)
IVR Avenger
+1  A: 

Singletons are appropriate for this scenario, BUT you have to make sure you're doing the singleton right.

So, for example, what Bozhno suggests is not a singleton, it's an ugly concoction of nasty statics that's not mockable, not easily testable, not injectable, and generally comes back to bite you in the ass.

An acceptable singleton is just your average class with one notable exception that it is guaranteed either by itself or by some external factory/framework (e.g Spring IoC) to exist in only one instance. If you go with the first approach, you do something like

private MyUberSingletonClass() {
    //..do your constructor stuff, note it's private
}

private static MyUberSingletonClass instance = null;

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

public String getUberUsefulStuff(){
    return "42";
}

That's acceptable if you don't really feel the need for a factory otherwise, and aren't using any IoC container in your app (good idea to think about using one though). Note the difference from Bozhno's example: this is a good vanilla class where the only static is an instance var and a method to return it. Also note the synchronized keyword required for lazy-initialization.

update: Pascal recommends this very cool post about a better way to lazy-init singletons in the comments below: http://crazybob.org/2007/01/lazy-loading-singletons.html

Paul Milovanov
You may want to read this: http://crazybob.org/2007/01/lazy-loading-singletons.html
Pascal Thivent
Very, very nice! Thanks Pascal. Much neater than the synchronized idiom. I'm just wondering whether this behavior is uniform for all JVMs since 1.3 -- i've been doing Blackberry devt lately, and as you know CLDC is a bitch.
Paul Milovanov
+2  A: 

As an alternative, did you consider using something like Apache Commons Configuration (or maybe another configuration framework)?

Pascal Thivent
(+1) it's good not to reinvent the wheel
Bozho
Yeah, that's basically my point.
Pascal Thivent
A: 
IVR Avenger
That is not exactly a singleton. You should have make an object of the WebServiceUrls an instance variable of itself and make the constructor private. The getInstance() method should check if the object has been created and create it if it has not before returning it.
Poindexter
Okay. But it mimics the structure detailed in Pascal's link, above. Is that not the approach that I should have taken?
IVR Avenger