Logging class in Qt library for foreign language calls

Question

I have a small C++/Qt library that interfaces with an application in another language (so interfacing must be performed using primitives, notably pointers to Handler objects). To implement debug logging, each Handler has its own debug file, managed by a Log object. These are both constructed in the initializeHandler function.

To make use of RAII, I didn't want Log objects to have a default constructor - they have a copy-constructor, and a constructor that takes a QString to open a QFile with. If the Log can't open the file, I want the initializeHandler function to fail, so I made it throw a LogExceptFailedOpen exception. To interact with the application using my library, I have to catch that exception and re-interpret it as a returned error code (I can't just let the exception propogate - the calling application can't catch it). But I want to use the Log object if construction succeeds. Which leads me to my problem.

If I declare and initialize the Log in the try block, I can't access it after the catch block. I can get around this by using a QSharedPointer<Log> with a scope larger than the try block, then using the Log copy-constructor to create another Log object afterwards (or I could just use the Log stored in logPtr, but that's not the point). However, this feels like a hack, and I feel that the code structure could instead be reworked somehow.

There's some discussion here around someone with a similar problem, which suggests that I should either

• do all my work within the try block, or
• abandon RAII for Log objects.

Neither option appeals to me very much. RAII is important for Log objects, as they get passed around and copied a lot and it would be a major hassle to look for all the places where they could go out of scope.

How can I improve my code to avoid this problem?

initializeHandler function:

typedef int ErrorCode;
#define ERROR_FILE_FAILED_OPEN   -12
#define ERROR_INVALID_ARGS       -1
#define ERROR_OK                 0

ErrorCode initializeHandler(Handler*& handler, const char * logFile, int x, int y)
{
    QSharedPointer<Log> logPtr;
    try {
        logPtr.reset(new Log(QString(logFile)));
    } catch (const LogExceptFailedOpen& e) {
        return ERROR_FILE_FAILED_OPEN;
    }
    Log logger(*logPtr);

    logger.log("Log started.");
    logger.log("Initializing handler...");

    Handler* temp;
    try {
        temp = new Handler(logger, x, y);
    } catch (const HandlerExceptInvalidArgs& e) {
        logger.log("Arguments invalid. x = " + QString::number(x)
                   + ", y = " + QString::number(y));
        return ERROR_INVALID_ARGS;
    }
    handler = temp;

    logger.log("Successfully initialized handler.");

    return ERROR_OK;
}

Log class:

class Log
{
private:
    QSharedPointer<QFile> file;
    QSharedPointer<QTextStream> stream;
public:
    void log(const QString& message)
    {
        (*stream) << (message + "\r\n");
    }

    // file constructor
    Log(const QString& fileName) : file(fileName)
    {
        if (!file->open(QFile::WriteOnly)) {
            throw LogExceptFailedOpen();
        }
        stream = QSharedPointer<QTextStream>(new QTextStream(file.data()));
    }

    // copy-constructor
    Log(const Log& other) : file(other.file), stream(other.stream) {}

}

Answer 1

It would be great to be able to discern what outside requirements and what are requirements that you are imposing because you are imagining a specific way to solve this problem. You don't post the Handler class. If you want a logger per Handler why not have the handler class create the logger ?

With regard to RAII, all objects destructors will be called at the end of the program, unless you need to open and close these files for other reasons, is there really an upside to make things more complicated than they already are ?

But yes that solution with the pointer and the copy doesn't look good, it smells like you trying to enforce a specific way of dealing with things that just doesn't fit the semantics of the code. The copy constructor in the logger is only there because of the way you want to create the logger, there doesn't seem to be any outside requirement that says that copy constructor is needed. If you need to detect two different error conditions from exceptions you can just throw two different exception types, when you create your handler.

Unless there is a specific reason for having the initializeHandler function you could as well handle most of the logic you have in that function in the constructor and do the errorhandling in various catch blocks. if you throw the exceptions in the constructor you don't have to check in the constructor if the preceding operations succeeded, that is one of the advantages of throwing exceptions for error handling. Also think about how often you expect the file open function to fail, if you expect this to be a regular occurrence then the handling would be different than if this is an 'exceptional' occurrence.

Here is one option how handler initialization could be handled.

QPointer<Handler> handler;
try {
    handler = new Handler(x, y, "Handler.txt");
  }
  catch (FileOpenException e) {
    // Handle file Open Exception
  }
  catch (HandlerException e) {
    // Handle Handler failure
  }
}

if (! handler.isNull())
{
  doProcessing();
}

Unless there are other requirements that force you to copy the logger, get rid of the copy constructor, then you don't have to use the QSharedPointer in your logger class. Instead you can just use plain stack members.