Sign in to follow this  
  • entries
    72
  • comments
    104
  • views
    45401

Citizen: Something on Singletons.

Sign in to follow this  

152 views

Citizen: Something on Singletons.

Before joining the Snow Hard team I hadn't worked with the singleton pattern at all. I had picked up stray pieces from an ongoing debate on whether or not to use it, but I hadn't picked up any facts about it. I tampered some with singleton classes during my work on Snow Hard and then when it was time to implement a logging class in Citizen it seemed like a good choice to make it a singleton (logging is pretty much the book example of using a singleton.) Anyway I wanted to know the downsides as well since they apparently had enough substance to keep a debate going. After googling around for a bit I found most of what I was looking for, pretty much summarized on this page by Alex Miller. The things people were most annoyed with were:

  • Singletons hide dependency - This one I saw coming with Snow Hard. The whole code base quickly became littered with calls to getInstance() and there was no way to see it until you actually read through the whole code. With 100+ code files, well, you simply don't.
  • Singletons are difficult to test - I can see the sense in this even though I haven't run into it myself yet. It can be difficult to inject a mock instance of a singleton if you implement it as a template for all types. The instance is created internally and to modify that you need to tamper with the template.
  • Singletons are hard to subclass - Since any subclass of a singleton would inherit the static creation code it would be hard to create an instance of the subclass.
  • Singletons may not stay single - It is possible, as Miller points out, that what begins as a singleton may need to become multiple later on. This can be difficult to address on a templated singleton class.

With these points in mind this is how my Log class turned out.

class Log {

friend class Loggable;

private:

std::ofstream *fout;
Log( );
~Log( );
void print( const std::string &s );
static Log &getInstance( );
};


class Loggable {

protected:

void logPrint( const std::string &s );

public:

Loggable( ) {}
~Loggable( ) {}
};

Log::Log( ) {
fout = new std::ofstream( "log.txt" );
}

Log::~Log( ) {
delete fout;
}

void Log::print( const std::string &str ) {
*fout << str << std::endl;
}

Log &Log::getInstance( ) {
static Log inst;
return inst;
}

void Loggable::logPrint( const std::string &s ) {
Log::getInstance( ).print( s );
}

Much like a singleton the Log class is statically instantiated by getInstance(). However all members of Log are private and the only way to access it is through the Loggable class which is a friend to Log. Any class that needs to log it's progress should inherit Loggable and use its logPrint(). logPrint() uses it's friend status to get an instance of Log and print to it. This addresses the dependency hiding since a class using the log must inherit Loggable.

I chose not to use the popular singleton<> template implementation because this is central to both the "testing" issue and the "multiple" issue. Without a template there isn't much harm in modifying Loggable to access a separate class from Log, say MockLog that behaves differently. Loggable could also be made support multiple simultaneous log types, and Log::getInstance() could be changed to support more than one instance without breaking the design.

I didn't manage to solve the subclassing issue however since it comes with the static instantiation. It's not a problem right now either, so I won't deal with it until I actually need to subclass Log or something with a similar design.

Sign in to follow this  


2 Comments


Recommended Comments

There is a potential gotcha here to be aware of though. Consider if you had this in another translation unit:

dog.h

#include "log.h"

class Dog : public Logger
{
public:
~Dog(){ Log::getInstance().Print("Dead dog"); }
};

extern Dog GlobalDog;


dog.cpp

#include "dog.h"

Dog GlobalDog;


Unfortunately it is quite possible that during program termination, the destructor of the static Log instance inside your Log::getInstance() method will be called prior to GlobalDog's destructor as they are both static objects and are defined in different translation units.

Unless you can guarantee that any instances of objects deriving from Logger and accessing fout will be destructed prior to the end of main() or WinMain(), you could have a problem.

If this is not feasible, I'd seriously consider, each time you need to log, reopening your log file in append mode, logging then closing it again. You could wrap this operation inside the constructor/destructor of a Log class and just declare a local instance when you need to log.

Share this comment


Link to comment

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now