2
\$\begingroup\$

I've created a simple logging system that I'm planning to use and I've tried out some modern C++ features to make it generic. First I've made a log class that represents a single log message:

struct log : std::stringstream
{
private:
    template <typename Level, size_t I = 0, typename... Stamps>
    static inline typename std::enable_if<I == sizeof...(Stamps), void>::type
        __stamp(log& l)
    {
    }

    template <typename Level, size_t I = 0, typename... Stamps>
    static inline typename std::enable_if<I < sizeof...(Stamps), void>::type
        __stamp(log& l)
    {
        l << std::tuple_element_t<I, std::tuple<Stamps...>>::Get<Level>();
        __stamp<Level, I + 1, Stamps...>(l);
    }

public:
    template <typename Level, typename... Stamps>
    static log& emit()
    {
        static log l;
        __stamp<Level, 0, Stamps...>(l);
        l << ": ";
        return l;
    }

    template <typename T>
    log& operator<<(T el)
    {
        *((std::stringstream*)this) << el;
        return *this;
    }

    template <typename Logger>
    void Flush()
    {
        Logger::Log(str().c_str());
        clear();
    }
};

template <typename Logger>
std::ostream& endlog(std::ostream& out)
{
    ((log*)&out)->Flush<Logger>();
    return out;
}

typedef Logger<ConsoleSink> DefaultLogger;
std::ostream& endlog(std::ostream& out)
{
    ((log*)&out)->Flush<DefaultLogger>();
    return out;
}

And I've created the main logger:

template <typename... Sinks>
class Logger
{
private:
    static std::tuple<Sinks...> s_Sinks;

private:
    template <size_t I = 0, typename... Tp>
    static inline typename std::enable_if<I == sizeof...(Tp), void>::type
        __Log(const char* msg)
    {
    }

    template <size_t I = 0, typename... Tp>
    static inline typename std::enable_if<I < sizeof...(Tp), void>::type
        __Log(const char* msg)
    {
        std::get<I>(s_Sinks).Write(msg);
        __Log<I + 1, Tp...>(msg);
    }

public:
    static void Log(const char* msg)
    {
        __Log<0, Sinks...>(msg);
    }
};

template <typename... Sinks>
std::tuple<Sinks...> Logger<Sinks...>::s_Sinks;

An example usage would be:

log::emit<Info, LevelStamp, SeparatorStamp, TimeStamp>() << "Hello, Logging " << 3 << endlog;

Now I can see a lot of problems with my own code, for example because of my emit function is defined with variadic stamps, there's no way to define a default overload for it so if I always want the same stamps, I have to type them out. Another is that my DefaultLogger is not configurable from outside the logger code which I think should be a feature.

Is my approach any good? How could I improve it?

\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Stop using double underscore.
The double underscore is reserved for the implementation it definitely should not be used in "user code".

See: What are the rules about using an underscore in a C++ identifier?

Recursive template Call.

You use a recursive technique to call the Write() method for all your sinks. I prefer to use the loop technique (I would say this is a more modern approach but I have no reference for that). But I find it clearer as you don't need the empty terminating function call.

private:
    using Tuple=std::tuple<Sinks...>;

    template<std::size_t... index>
    static void writeToLogs(char const* m, std::integer_sequence<std::size_t, index...> const&)
    {
        auto ignore = {(std::get<index>(s_Sinks).Write(m),true)...};
    }

public:
    static void Log(const char* msg)
    {
        writeToLogs(msg, std::make_index_sequence<std::tuple_size<Tuple>::value>());
    }

Design

Everybody and their grandson tends to want to write a logger at some point in time. The trouble is we already have a good logger built into the *nix world (syslog). If you are not using syslog to do your logging you are doing it wrong.

It (syslog) already does everything you need to do and more and there is a whole ecosystem built around it for monitoring. Now there is nothing wrong with writing a wrapper around syslog but you have to be smart about it (the current ones are very smart).

Log(debug, getSomeMessage());

This should not call the function getMessage() if the debugging level is set to production. That would be a complete wast of time and resources as debug messages are currently being ignored.

\$\endgroup\$
2
\$\begingroup\$
  • The APIs you wrote could be tightened. Here is something bad that can be done with them:

    std::ofstream out{ "somefile.txt" };
    out << endlog; // bad code
    
  • __stamp
    

    This is a bad name. Depending on how you (and everyone else using this API) write client code, you may end up with conflicts between these APIs and standard library implementation APIs.

    As a rule, do not use symbols starting with an underscore, and especially not with underscores followed by a capital letter. They are reserved for standard library implementations by convention and you are violating the principle of least surprise.

  • log::emit<Info, LevelStamp, SeparatorStamp, TimeStamp>() << ...
    

    This is cumbersome to write out, and it will be cumbersome to maintain.

    Try following these guidelines:

    • if you have to write (mostly) the same line of code over and over again, extract it into a function (it will be one order of magnitude easier to maintain and change).

    • if you have code that should be extracted into a function, do so from the beginning (make it part of your logging API):

      namespace diagnostics // logging namespace example
      {
          inline auto info() {
              return log::emit<Info, LevelStamp, SeparatorStamp, TimeStamp>();
          }
          inline auto warn() {
              return log::emit<Warn, LevelStamp, SeparatorStamp, TimeStamp>();
          }
          inline auto error() {
             return log::emit<Error, LevelStamp, SeparatorStamp, TimeStamp>();
          }
      }
      

    client code:

    using endl = diagnostics::endlog;
    diagnostics::info() << "Hello, Logging " << 3 << endl;
    diagnostics::error() << "Mistakes were made ..." << endl;
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.