CodeSOD: Single or Mingle

Singletons is arguably the easiest to understand design pattern, and thus, one of the most frequently implemented design patterns, even- especially– when it isn’t necessary. Its simplicity is its weakness.

Bartłomiej inherited some code which implemented this pattern many, many times. None of them worked quite correctly, and all of them tried to create a singleton a different way.

For example, this one:

<span class="hljs-keyword">public</span> <span class="hljs-keyword">class</span> <span class="hljs-title">SystemMemorySettings</span>
{
    <span class="hljs-keyword">private</span> <span class="hljs-keyword">static</span> SystemMemorySettings _instance;

    <span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-title">SystemMemorySettings</span>()</span>
    {
        <span class="hljs-keyword">if</span> (_instance == <span class="hljs-literal">null</span>)
        {
            _instance = <span class="hljs-keyword">this</span>;
        }
    }

    <span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-keyword">static</span> SystemMemorySettings <span class="hljs-title">GetInstance</span>()</span>
    {
        <span class="hljs-keyword">return</span> _instance;
    }

    <span class="hljs-function"><span class="hljs-keyword">public</span> <span class="hljs-keyword">void</span> <span class="hljs-title">DoSomething</span>()</span>
    {
    ...
        <span class="hljs-comment">// (this must only be done for singleton instance - not for working copy)</span>
        <span class="hljs-keyword">if</span> (<span class="hljs-keyword">this</span> != _instance)
        {
            <span class="hljs-keyword">return</span>;
        }
    ...
    }
}

The only thing they got correct was the static method which returns an instance, but everything else is wrong. They construct the instance in the constructor, meaning this isn’t actually a singleton, since you can construct it multiple times. You just can’t use it.

And you can’t use it because of the real “magic” here: DoSomething, which checks if the currently active instance is also the originally constructed instance. If it isn’t, this function just fails silently and does nothing.

A common critique of singletons is that they’re simply “global variables with extra steps,” but this doesn’t even succeed at that- it’s just a failure, top to bottom.

[Advertisement]
Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

Source: Read More