views:

162

answers:

3

I've inherited a project that stores various parameters either in a config file, the registry and a database. Whoever needs one of these parameters just reads (and in some cases writes) it directly from the store. This, or course, is stupid, so my first thought was to refactor the existing code so that the client doesn't know where the parameter is stored in. I created a classic AppSettings class that has a property for each parameter. Since the store has to have global scope I made a thread-safe singleton. The class doesn't store the parameter values in fields but rather acts as an access point by reading and writing them to and from the actual store, be it config file, registry or database. These days it's hard to avoid all the talk about the dangers of singletons and global state. I will take a proper look at dependency injection and Spring etc later, but for now, I just have a couple of questions.

1) What type of problems, other than testability, can you see with my solution?
2) What would be a light weight alternative? Creating a factory for each object that uses the parameters is not an option (too much work).
3) Wouldn't using a singleton serve as an acceptable compromise until I have a chance to do some heavier refactoring?
4) If the properties in my singleton class only had getters, would that make it OK?

I can anticipate that the store for some of the parameters will change in the future (eg. from registry to database), so that was my motivation for hiding the store behind a singleton class.

A: 

I would challenge the assumption that since the config data is global, that you need a global singleton to access it, especially for reading. Consider creating an AppSettings class that can be invoked as needed to read your config settings.

If you need to write in a thread-safe manner, you can create a static (or singleton) private member of the AppSettings class to control writing only. Thus any instance of AppSettings can write, but the "global" access is actually restricted to the AppSettings class.

Doug
+1  A: 

This is a bit of a non-answer, but I highly recommend the c2wiki's pages on Singletons as a reference http://c2.com/cgi/wiki?search=Singleton

And also the page http://c2.com/cgi/wiki?GlobalVariablesAreBad

I think the general verdict is that global state creates coupling between vastly different parts of your system that must be thought about and designed around very carefully. The question is, are all of those settings truly global and needed by disparate parts of the system? If not, then is there any way to separate them into smaller parts that can live inside different modules at a lower access level?

If it's a small project I wouldn't worry too much about it, but there is a lot of wisdom on those c2wiki pages about global state and singletons being a pain for larger projects.

alt_tab
+1! See also http://blogs.msdn.com/scottdensmore/archive/2004/05/25/140827.aspx I'd suggest taking a proper look at dependency injection sooner rather than later.
TrueWill
A: 

Thanks, guys. I would consider this a medium-size project (about 200KLOC) and it's C#. The problem is that the project has a long and troubled history and a lot coders have worked on it. As much as I'd like to properly learn dependency injection (as I do understand and subscribe to the concept), the deadline is closing fast so now is not the time for it. After looking at my current singleton class I decided to split it into two instance classes. Some of the parameters are used all over but some only in a single assembly. And like Doug said, I can achieve thread-safety just as easily with an instance class.

As for the various dependency injection frameworks, the problem is there are too many. I've briefly looked at Spring and Unity. I wish I could find a summary of the differences.

Thanks again!

Rubio
I think you should edit this answer into your question, since it is not really an answer.
Lucas