Log Library (Mini-Review?)

Hey, I have been coding a 30 minutes logging library for my applications, I would appreciate some comments about it .

Thank You!

Code:

#include <iostream>
#include <string>
#include <cassert>
#include <fstream>
#include <functional>

namespace log_level {

enum type
{
  ok,
  info,
  warning,
  error
};

const char* get_string(type level)
{
  switch (level)
  {
  case ok:      return "OK";
  case info:    return "INFO";
  case warning: return "WARNING";
  case error:   return "ERROR";
  }
  assert(false);
  return "";
}

};

template<class Char, class Traits = std::char_traits<Char> >
class basic_log_message
{
  typedef Char char_t;

  const char_t* m_message;
  log_level::type m_level;
public:
  const char_t* get_message() const { return m_message; }
  log_level::type get_level() const { return m_level;   }

  basic_log_message(const char_t* message, log_level::type level = log_level::ok)
    : m_message(message)
    , m_level(level)
  {
  }
};

typedef basic_log_message<wchar_t, std::char_traits<wchar_t> > wlog_message;
typedef basic_log_message<char,    std::char_traits<char>    >  log_message;

template<class Char>
struct basic_formatter;

template<class Char, class Traits = std::char_traits<Char>, class Formatter = basic_formatter<Char> >
class basic_logger
{
  typedef Char char_t;
  typedef basic_logger<Char, Traits, Formatter> my_type;
  typedef basic_log_message<Char, Traits> message_t;

  // we use reference_wrapper so that we can reassign the output reference
  std::tr1::reference_wrapper<std::basic_ostream<Char, Traits> > m_output;
protected:
  // unformatted logging
  template<class T>
  my_type& operator<<(const T& value)
  {
    m_output.get() << value;
    return *this;
  }
public:
  basic_logger(std::basic_ostream<Char, Traits>& output)
    : m_output(std::tr1::ref(output))
  {
  }

  void redirect(std::basic_ostream<Char, Traits>& output)
  {
    m_output = std::tr1::ref(output);
  }

  void put(const message_t& message)
  {
    Formatter::append_message(*this, message);
  }

  void put(const char_t* message)
  {
    put(log_message(message));
  }

  friend typename Formatter;
};

typedef basic_logger<wchar_t, std::char_traits<wchar_t> > wlogger;
typedef basic_logger<char,    std::char_traits<char>    >  logger;

extern wlogger wlog(std::wcout);
extern  logger  log(std::cout );

template<class Char, class Traits>
basic_logger<Char, Traits>& operator<<(basic_logger<Char, Traits>& other_logger, const basic_log_message<Char, Traits>& message)
{
  other_logger.put(message);
  return other_logger;
}

template<class Char>
struct basic_formatter
{
};

template<>
struct basic_formatter<char>
{
  template<class Traits>
  static void append_message(basic_logger<char, Traits, basic_formatter<char> >& logger, const basic_log_message<char, Traits>& message)
  {
    logger << '[' << log_level::get_string(message.get_level()) << "] " << message.get_message() << "\n";
  }
};

template<>
struct basic_formatter<wchar_t>
{
  template<class Traits>
  static void append_message(basic_logger<wchar_t, Traits, basic_formatter<wchar_t> >& logger, const basic_log_message<wchar_t, Traits>& message)
  {
    logger << L'[' << log_level::get_string(message.get_level()) << L"] " << message.get_message() << L"\n";
  }
};

void main()
{
  std::ofstream file("log.txt");
  log.redirect(file);
  log << log_message("teste") << log_message("kk", log_level::error);
  wlog << wlog_message(L"teste2");
}

I'm no good at template coding, but I would say that this:

Code:

  switch (level)
  {
  case ok:      return "OK";
  case info:    return "INFO";
  case warning: return "WARNING";
  case error:   return "ERROR";
  }
  assert(false);
  return "";

probably should have all the level strings of the same length - it makes the rest of the output easier to read. You may then want to shorten WARNING to WARN to save a few characters, and perhaps also ERROR to ERR.

Also, why not put the assert(false) and return "" in a default: ?

--
Mats

Originally Posted by matsp

I'm no good at template coding, but I would say that this:

Code:

  switch (level)
  {
  case ok:      return "OK";
  case info:    return "INFO";
  case warning: return "WARNING";
  case error:   return "ERROR";
  }
  assert(false);
  return "";

probably should have all the level strings of the same length - it makes the rest of the output easier to read. You may then want to shorten WARNING to WARN to save a few characters, and perhaps also ERROR to ERR.

You are right, thank you.

Originally Posted by matsp

Also, why not put the assert(false) and return "" in a default: ?

Oh, no special reason, but that should really be a little more readable, thank you for
pointing out that.

Besides that did you like the interface that the user has to deal with to use the library? I found the only good way to put severity messages is to create an object like I did there.

I think the severity stuff is fine.

Two things that come to mind:
1. Can you do something like this:

Code:

   int somevalue = somefunction();
   if (somevalue != 42)
       log << logmessage("Expected", log_level::error) << 42 << " got " << somevalue;

[As I said, I'm not very at home in template-speak - so this may be just me not being able to read templates]

The other thing is a "filter" to block messages below a certain logging level - e.g. only show error messages, not warning or info.

--
Mats

Originally Posted by matsp

I think the severity stuff is fine.

Two things that come to mind:
1. Can you do something like this:

Code:

   int somevalue = somefunction();
   if (somevalue != 42)
       log << logmessage("Expected", log_level::error) << 42 << " got " << somevalue;

Yes, that is perfectly doable, thanks for the idea.

Originally Posted by matsp

[As I said, I'm not very at home in template-speak - so this may be just me not being able to read templates]

No problem

Originally Posted by matsp

The other thing is a "filter" to block messages below a certain logging level - e.g. only show error messages, not warning or info.

Oh, a filter is good too, thank yoou

As soon as i make these changes I will post them here .

Well, as i didn't enjoyed the idea of having a object to represent a message I refactored it this way:

Code:

#include <iostream>
#include <string>
#include <cassert>
#include <fstream>
#include <functional>

namespace log_level {

enum type
{
  ok,
  info,
  warning,
  error
};

const char* get_string(type level)
{
  switch (level)
  {
  case ok:      return "SUCC";
  case info:    return "INFO";
  case warning: return "WARN";
  case error:   return "ERRO";
  default:
    assert(false);
    return "";
  }
}

};

template<class Char>
struct basic_formatter;

template<class Char, class Traits = std::char_traits<Char>, class Formatter = basic_formatter<Char> >
class basic_logger
{
  typedef Char char_t;
  typedef basic_logger<Char, Traits, Formatter> my_type;
  typedef std::basic_ostream<Char, Traits> ostream_t;

  // we use reference_wrapper so that we can reassign the output reference
  std::tr1::reference_wrapper<ostream_t> m_output;
protected:
  ostream_t& get_stream() const { return m_output; }
public:
  basic_logger(std::basic_ostream<Char, Traits>& output)
    : m_output(std::tr1::ref(output))
  {
  }

  void redirect(std::basic_ostream<Char, Traits>& output)
  {
    m_output = std::tr1::ref(output);
  }

  void put(const log_level::type& level)
  {
    Formatter::append_level(*this, level);
  }
  template<class Char, class Traits>
  friend std::basic_ostream<Char, Traits>& operator<<(basic_logger<Char, Traits>& other_logger, log_level::type level);
  friend typename Formatter;
};

typedef basic_logger<wchar_t, std::char_traits<wchar_t> > wlogger;
typedef basic_logger<char,    std::char_traits<char>    >  logger;

extern wlogger wlog(std::wcout);
extern  logger  log(std::cout );

template<class Char, class Traits>
std::basic_ostream<Char, Traits>& operator<<(basic_logger<Char, Traits>& other_logger, log_level::type level)
{
  other_logger.put(level);
  return other_logger.get_stream();
}

template<class Char>
struct basic_formatter
{
  template<class Traits>
  static void append_level(basic_logger<Char, Traits, basic_formatter<Char> >& logger, const log_level::type& level)
  {
    logger.get_stream() << '[' << log_level::get_string(level) << "] ";
  }
};

void main()
{
  log << log_level::error << "WOOPS " << 14 << "?\n";
  std::cin.get();
}