HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Logging object for C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
loggingobjectfor

Problem

I've been always updating my log methods, because I started SDL and I needed some new methods. First, I made difference between debug and release, and so console and file. But now I want a more powerful method, which handles the console with implicating the file, and works like C++ streams. I would like to know if it's a good design.

#ifndef LOGGING_INCLUDED
#define LOGGING_INCLUDED

#include 
#include 
#include 
#include 

namespace BSSE_System
{
    /*bool addStream(std::ostream* os);
    bool addStream(std::ostream* os, unsigned short& channel);

    void doLog(const unsigned short& channel, const std::string& message, bool doTime = true);*/ //on specific channel;
    /*void doLog(const std::string& message, bool doTime = true);*/ //on all channels

    class LogObject
    {
    public:
        LogObject()
        {
            streams.resize(0);
            doTime = true;
        }

        bool addStream(std::ostream* os)
        {
            if (os == NULL)
            {
                return false;
            }
            else
            {
                streams.push_back(os);
                return true;
            }
        }

        bool addStream(std::ostream* os, unsigned short& channel)
        {
            if (os == NULL)
            {
                return false;
            }
            else
            {
                streams.push_back(os);
                channel = streams.size() - 1;
                return true;
            }
        }

        void set_printTime(const bool& state)
        {
            doTime = state;
        }

        template 
        friend LogObject& operator::iterator it = lo.streams.begin();
                 it tm_hour tm_min tm_sec tm_hour
                    tm_min tm_sec  streams;
        bool doTime;
        bool timeSwitch;
    };

    extern LogObject logger;
}

#endif // LOGGING_INCLUDED


extern LoggerClass logger; //defined in cpp like for std's cout and cin


So, is this a good, safe m

Solution

I see some things that may help you improve your code.

Use smart pointers

The design as currently implemented has a serious problem -- the raw pointers being stored might not actually point to live objects when output is attempted. To fix this, the code should instead std::shared_ptr rather than raw pointers.

Be careful with template bloat

The way the current operator<< object is written, a new function will be generated for every different class that is logged. In other words, this code:

log << "mystery";
log << 42;
log << 98.6;


Would cause three different versions of that code to be generated. If you are only logging a few types, that might not be bad, but I'd recommend instead having the logger take a const std::string& as an argument and having the caller do whatever is required to turn the message into a string.

Use std::put_time

If your compiler is C++11 compliant, you can use std::put_time and make your code considerably simpler. Instead of this:

**it tm_hour tm_min tm_sec << " - ";


You can write this:

**it << std::put_time(timeStruct, "%T - ");


Use "range-for" to simplify your code

Instead of using explicit iterators, you can use "range-for" and simplify your code. For example, the operator<< code could be written like this:

template 
friend LogObject& operator<< (LogObject& lo, const T& message)
{
    for (auto &out : lo.streams) {
        if (lo.doTime) {
            time_t rawTime = time(NULL);
            tm * timeStruct = localtime(&rawTime);
            *out << std::put_time(timeStruct, "%T - ");
        }
        *out << message;
    }
    return lo;
}


Move loop invariants outside the loop

In the operator << code above note that the time is still being recalculated every loop iteration. Unless IO is really slow or you have lots of output log streams or both, this probably isn't necessary. Instead, you could calculate time once and then use the precalculated time string. I'd write it like this:

template 
friend LogObject& operator<< (LogObject& lo, const T& message)
{
    std::string timestring = "";
    if (lo.doTime) {
        time_t rawTime = time(NULL);
        tm * timeStruct = localtime(&rawTime);
        std::stringstream tstr;
        tstr << std::put_time(timeStruct, "%T - ");
        timestring = tstr.str();
    }
    for (auto &out : lo.streams) {
        *out << timestring << message << '\n';
    }
    return lo;
}

Code Snippets

log << "mystery";
log << 42;
log << 98.6;
**it << std::setw(2) << timeStruct->tm_hour << ':'
    << std::setw(2) << timeStruct->tm_min << ':'
    << std::setw(2) << timeStruct->tm_sec << " - ";
**it << std::put_time(timeStruct, "%T - ");
template <typename T>
friend LogObject& operator<< (LogObject& lo, const T& message)
{
    for (auto &out : lo.streams) {
        if (lo.doTime) {
            time_t rawTime = time(NULL);
            tm * timeStruct = localtime(&rawTime);
            *out << std::put_time(timeStruct, "%T - ");
        }
        *out << message;
    }
    return lo;
}
template <typename T>
friend LogObject& operator<< (LogObject& lo, const T& message)
{
    std::string timestring = "";
    if (lo.doTime) {
        time_t rawTime = time(NULL);
        tm * timeStruct = localtime(&rawTime);
        std::stringstream tstr;
        tstr << std::put_time(timeStruct, "%T - ");
        timestring = tstr.str();
    }
    for (auto &out : lo.streams) {
        *out << timestring << message << '\n';
    }
    return lo;
}

Context

StackExchange Code Review Q#121454, answer score: 7

Revisions (0)

No revisions yet.