Search code examples
c#design-patternssingletonstatic-classes

How to refactor a singleton class and avoid doing the same mistakes again


I've started a small application in WPF and used a Singleton class to handle all the application logic. Plus I had a few ObservableCollections there that are binded to DataGrids on the View.

The problem: what was supposed to be a small program started to grow in functionality and the code is now too hard to maintain, reuse and there is a high level of code coupling.

And so I've started to move the code to other classes. For example, I've a class that just handles the File readings. I've made this class static because I just run those methods once (when I need to import data to a database), and when they are finished I don't need those objects anymore and just forget that they exist.

Now I'm considering doing the same for other methods, like the ones that retrieve data form the database.

My doubt is if that is that the correct way to solve the problem? I'm afraid that the use of a static classes will be like a multiplication of singletons.


Solution

  • Static classes are considered evil by some people, but that is just an opinion. When I have these questions, I take a look at the .NET-framework: How is it solved inside there?

    Sometimes a singleton can be refactored to a static class. It depends on the situation. If your singleton is of a type that inherits (read: must inherit) other classes or interfaces, it cannot be converted to a static class, since a static class cannot inherit anything.

    If you create a static class, try to obey the following rules: (These rules are also obeyed by the .NET framework):

    • All static members must be thread safe.

    That's it! :)

    The rule sounds simple, but implies a lot:

    • All static members work independent of each other. So one call will never affect the result of another call.
    • A static class is not allowed to maintain a (static) state.
    • If the class has static fields, make sure they are readonly or const. Make sure the content of those fields never changes.

    Of course there could be some little exceptions. For example: A static class could maintain an internal dictionary for caching results. Modifying this cache must be thread safe. Since it is internal stuff, for the outside world the static class still obeys the rules stated above.

    So... in short: If your singleton is NOT thread safe (holds state, etc.) do NOT convert it to a static class.

    * EDIT *

    Using a singleton normally means you have a static property containing one instance of a certain type. Since this is a static property it also must obey above rules, which means that the instance must be thread safe.

    If your (singleton) instance is not thread safe, redesign your application so it does not use this singleton or static class. Let all code create a new instance of this class when needed.