Singleton class across the whole project approach

Question

I have a singleton class for logging purpose in my Qt project. In each class except the singleton one, there is a pointer point to the singleton object and a signal connected to an writing slot in the singleton object. Whichever class wants to write log info just emit that signal. The signals are queued so it's thread-safe.

Please critique this approach from OOP point of view, thanks.

============================================================================================= Edit 1: Thank you all your applies, listening to opposite opinions is always a big learning.

Let me explain more about my approach and what I did in my code so far: Exactly as MikeMB pointer, the singleton class has a static function like get_instance() that returns a reference to that singleton. I stored it in a local pointer in each class's constructor, so it will be destroyed after the constructor returns. It is convenient for checking if I got a null pointer and makes the code more readable. I don't like something as this:

if(mySingletonClass::gerInstance() == NULL) { ... }
connect(gerInstance(), SIGNAL(write(QString)), this, SLOT(write(QString)));

because it is more expensive than this:

QPointer<mySingletonClass>  singletonInstance = mySingletonClass::getInstance();
if(singletonInstance.isNull) { ... }
connect(singletonInstance, SIGNAL(write(QString)), this, SLOT(write(QString)));

Calling a function twice is more expensive than creating a local variable from ASM's point of view because of push, pop and return address calculation.

Here is my singleton class:

class CSuperLog : public QObject
{
    Q_OBJECT

public:
        // This static function creates its instance on the first call 
        // and returns it's own instance just created
        // It only returns its own instance on the later calls
        static QPointer<CSuperLog> getInstance(void); // 
        ~CSuperLog();

public slots:
    void writingLog(QString aline);

private:
    static bool ready;
    static bool instanceFlag;
    static bool initSuccess;
    static QPointer<CSuperLog> ptrInstance;

    QTextStream * stream;
    QFile * oFile;
    QString logFile;

    explicit CSuperLog(QObject *parent = 0);
};

I call getInstance() at the beginning of main() so make sure it is read immediately for each other class whenever they need to log important information.

MikeMB:

Your approach is making a middle man sitting in between, it makes the path of the logging info much longer because the signals in Qt are always queued except you make direct connection. The reason why I can't make direct connection here is it make the class non-thread-safe since I use threads in each other classes. Yes, someone will say you can use Mutex, but mutex also creates a queue when more than one thread competing on the same resource. Why don't you use the existing mechanism in Qt instead of making your own?

Thank you all of your posts!

=========================================================

Edit 2:

To Marcel Blanck:

1. I like your approach as well because you considered resource competition.
2. Almost in every class, I need signals and slots, so I need QObject, and this is why I choose Qt.
3. There should be only one instance for one static object, if I didn't get it wrong.
4. Using semaphores is same as using signals/slots in Qt, both generates message queue.
5. There always be pros and cons regarding the software design pattern and the application performance. Adding more layers in between makes your code more flexible, but decreases the performance significantly on those lower-configured hardware, making your application depending one most powerful hardware, and that's why most of modern OSes are written in pure C and ASM. How to balance them is really a big challenge.

Could you please explain a little bit more about your static Logger factory approach? Thanks.

Answer 1

I would consider separating the fact that a logger should be unique, and how the other classes get an instance of the logger class.

Creating and obtaining an instance of the logger could be handled in some sort of factory that internally encapsulates its construction and makes only one instance if need be.

Then, the way that the other classes get an instance of the logger could be handled via Dependency injection or by a static method defined on the aforementioned factory. Using dependency injection, you create the logger first, then inject it into the other classes once created.

Answer 2

I do not like singletons so much because it is always unclean to use them. I have even read job descriptions that say "Knowledge of design patterns while knowing that Singleton isn't one to use". Singleton leads to dependecy hell and if you ever want to change to a completely different logging approach (mabe for testing or production), while not destroying the old one you, need to change a lot.

Another problem with the approch is the usage of signals. Yes get thread savety for free, and do not interrupt the code execution so much but...

• Every object you log from needs to be a QObject
• If you hunt crashes your last logs will not be printed because the logger had no time to do it before the program crashed.

I would print directly. Maybe you can have a static Logger factory that returns a logger so you can have one logger object in every thread (memory impact will still be very small). Or you have one that is threadsave using semaphores and has a static interface. In both cases the logger should be used via an interface to be more flexible later.

Also make sure that your approach prints directly. Even printf writes to a buffer before being printed and you need to flush it every time or you might never find crashes under bad circumstances, if hunting for them.

Just my 2 cents.

Answer 3

A singleton usually has a static function like get_instance() that returns a reference to that singleton, so you don't need to store a pointer to the singleton in every object.

Furthermore it makes no sense, to let each object connect its log signal to the logging slot of the logging object itself, because that makes each and every class in your project dependent on your logging class. Instead, let a class just emit the signal with the log information and establish the connection somewhere central on a higher level (e.g. when setting up your system in the main function). So your other classes don't have to know who is listening (if at all) and you can easily modify or replace your logging class and mechanism.

Btw.: There are already pretty advanced logging libraries out there, so you should find out if you can use one of them or at least, how they are used and adapt that concept to your needs.

==========================

EDIT 1 (response to EDIT 1 of QtFan):

Sorry, apparently I miss understood you because I thought the pointer would be a class member and not only a local variable in the constructor which is of course fine.

Let me also clarify what I meant by making the connection on a higher level: This was solely aimed at where you make the connection - i.e. where you put the line

connect(gerInstance(), SIGNAL(write(QString)), this, SLOT(write(QString)));

I was suggesting to put this somewhere outside the class e.g. into the main function. So the pseudo code would look something like this:

void main() {
    create Thread1
    create Thread2
    create Thread3

    create Logger

    connect Thread1 signal to Logger slot
    connect Thread2 signal to Logger slot
    connect Thread3 signal to Logger slot

    run Thread1
    run Thread2
    run Thread3
}

This has the advantage that your classes don't have to be aware of the kind of logger you are using and whether there is only one or multiple or no one at all. I think the whole idea about signals and slots is that the emitting object doesn't need to know where its signals are processed and the receiving class doesn't have to know where the signals are coming from.

Of course, this is only feasible, if you don't create your objects / threads dynamically during the program's run time. It also doesn't work, if you want to log during the creation of your objects.