Singleton As Band-Aid

In my last job, there was an internal site on which employees could create their own personal work-related blogs. I had one that dealt with C++ and OO topics that I'd been irritated into writing about in production code: I had to leave it behind when I left, but transferred the site ownership to my team leader so that it at least wouldn't be cleaned out automatically. My current workplace has various ways for teams to share data but nothing comparable for individuals.

This means that anything I'm irritated into will end up being posted here. Any code examples will be generalized enough to be generic and no details of proprietary IP will be shared, although various issues of actual production code quality  may be referenced, from this or from prior work contexts, without any specification of where.

For anyone who is not a programmer, these are likely to be TL;DR.

In the nearly thirty years since Design Patterns came out, the Singleton has generally been recognized as an anti-pattern, encouraging the use of global data with a mask on and hiding the flow of control, to say nothing of complicating unit tests.

However, in the context of trying to convert legacy code into well-designed and tested code, the Singleton has a role to play as a Band-Aid (sticking plaster to right-pondians) as a temporary measure, even though its ultimate fate will be un-Singletoning itself.

Consider the case where we are converting a legacy method with some associated data into a proper class. It uses ambient data from the huge source file it is being pulled out of freely, but the data it uses is tightly coupled in a logical sense, and if you group it together it naturally attracts a few existing functions which use that data and little else, to make a nicely encapsulated class. The data tracks state of ene sort or another and notionally has to be unique.

// in file RecordTracker.h
class Record;

class RecordTracker
{
public:
    void addRecord(const TransactionId& inId, const Record& inNewRecord);

    const Record& getRecord(const TransactionId& inId) const;
};

// In file f.cpp
RecordTracker theRecordTracker;

class RecordUser
{
public:
   void processTransactionUpdate(const Transaction& inTransaction)
   {
       //...
       const Record& rec = theRecordTracker.getRecord(inTransaction.getId();
       //...
   }
};

The data is used in several other compilation units, where it was declared extern. We update all of those locations to use the new class.

Now our legacy method uses one object, which it is still reading from global data. If we extract an interface from the new object, we can pass it in to the object we are now building on construction, as an instance of simple dependency injection. So we do that.

// in file IRecordTracker.h
class Record;

class IRecordTracker
{
public:

    virtual ~IRecordTracker();
    
    virtual void addRecord(const TransactionId& inId, const Record& inNewRecord) = 0;

    virtual const Record& getRecord(const TransactionId& inId) const; = 0;
};

// in file RecordTracker.h
#include "IRecordTracker.h"

class RecordTracker: public IRecordTracker
{
public:

    virtual ~RecordTracker();
    
    virtual void addRecord(const TransactionId& inId, const Record& inNewRecord);

    virtual const Record& getRecord(const TransactionId& inId) const;
};

// in file RecordUser.h
class RecordUser
{
public:

   RecordUser(const IRecordTracker& inTracker): m_tracker(inTracker) { }

   void processTransaction(const Transaction& inTransaction)
   {
       //...
       const Record& rec = m_tracker.getRecord(inTransaction.getId();
       //...
   }
private:
   const IRecordTracker& m_tracker;
};

There is a remaining problem: the object in question is still global data, used in multiple places. In fact, the second class we extracted the function into turns out to be useful in some (but not all) of those cases, so it's getting the value passed in there via interface as well.

The original location might be tolerable - the data is static lifetime data in that place. It could even in theory be moved into the compilation unit for the second class as an implementation detail - but we still have all the other access points to look at.

Some of those access points have very long call chains before they go back to a common source, frequently main(), with narrow interfaces[1]. Many of them have little to do with the area we're working on aside from incidental use of the data in question, and it's not in our scope to do far-flung imperial refactoring beyond the area we're having to touch. So we have two options:

1. Leave it as it is; at least the class we're worried about is now functioning in a testable manner.
2. Convert the instance of the data into a Singleton and have all the places which currently use the concrete class use the Singleton Instance() method. (Don't actually change the data class: implement the Singleton to use the interface and delegate operations to the original substantive class. This doesn't mess with our testing and prepares for the end of the process when the Singleton goes away.)

#include "RecordTracker.h"

class SingletonRecordTracker: public IRecordTracker
{
public:

    static IRecordTracker& Instance();
    
    virtual ~SingletonRecordTracker();
    
    virtual void addRecord(const TransactionId& inId, const Record& inNewRecord)
    {
        m_delegate.addRecord(inId, inNewRecord);
    }

    virtual const Record& getRecord(const TransactionId& inId) const
    {
        return m_delegate.getRecord(inId);
    }

private:
   SingletonRecordTracker();
   RecordTracker m_delegate;
};

If we do the second, we have a clear marker for the issue. We can also push the actual invocation up our call stack for the immediate case we're dealing with, widening interfaces to accept the interface, until we get to the point where the instance "naturally" lives (frequently in main() if it's dependent in any way on program parameters).

In the future, refactorings can push up the calling level from other locations using the interface higher up the call tree. As we go, we may (probably will) end up with more classes which hold a reference to the interface, as well. Eventually, all the calls invoking the Singleton will be in the same context.

At that point, we get rid of the Singleton and replace it with a simple instance of the class. (If the program is resolutely single-threaded, it may even be possible to allocate it on the stack; otherwise it will have to be static or allocated via operator new.)

So the Singleton acts as a temporary way of patching the problem for a period of time, better than the original state, and also as a marker that the problem continues to exist. (The analogy here is not flesh-coloured or clear Band-Aids, but the ones with pictures of Dora the Explorer I used to get for my daughter.) The existence of those Instance() calls is an explicit and deliberate marker of what has to be fixed, and where.

[1]One application I know, originally C but mixed C/C++ for over 20 years, heavily used, passes all execution of transactions through a single function which takes one argument - an enum indicating the type of operation to take.  This logic is in shared code and is called in several different applications. Someday this will presumably be refactored, but until then it's a barrier to any attempts to get rid of global data in some form.