Support Logging to any std::ostream
Currently, you only log to stdout
from the C standard library. It would be better to tell the logger which file or stream to output to when it is constructed, defaulting to std::cerr
or perhaps std::clog
. You can instead construct a logger that logs to a file, by passing it a std::ofstream&
.
Use C++ Style I/O, or std::format
Currently, the interface is a C-style format string and variadic arguments, but without the checks the compiler does for correctness. This is obsolete and finicky.
If you accept a std::string_view
, you can pass in any string-like object, including the return value of std::format
. You probably want an additional overload for const char*
, to cover all your bases. It also guarantees that each log entry is a single output call, which (as G. Sliepen reminded me), is already thread-safe.
Alternatively, you can overload operator<<
to accept any type that is overloaded to output to an ostream&
.
Use std::source_location
Instead of C-style Macros
A better solution than a C-style macro for passing the logger the source line that calls it is std::source_location
. Give the logging function a default final argument const std::source_location srcl = std::source_location::current()
. Then, the compiler will default to passing the function the source location of the call site.
Also Replace the Other C-Style Macros
In general, use macros only when you have to. A good solution here would be to replace the macros that do formatting with wrappers that return a std::string
object from std::format
or std::stringstream
, and pass (or maybe move) it to the logger’s output function in a tail-call. The string will automatically be destroyed when the logger is done with it and returns.
Use the Mutex
Having the threads claim a mutex to be able to log to the stream is a great idea, but threads need to actually lock the mutex before doing I/O.
If you are writing to a stream that something other than the log object might write to, it could be worthwhile to pass the constructor a reference to the mutex it should be using. That way, other functions can also lock the same mutex and interoperate with the logger. By default, the logger can still create its own mutex, which only it uses.
Are You Sure About the Private and Deleted Constructors?
There doesn’t appear to be a way to actually create one of these. If it’s meant to be a base class, the private:
constructors should be protected:
, so daughter classes can implicitly call them, and also should have a protected:
, virtual
destructor. If this is meant to be instantiated, it needs at least one public:
constructor. It probably makes sense for there to be a default constructor as well, since there are sensible defaults.
Consider as well whether you actually want to delete
the copy constructor. You say that you want this to be used from multiple threads, and although they might all access it through a std::shared_ptr
or reference, you might legitimately want to pass each thread its own copy. That way, the object only needs to serialize the output operation itself, not all access from any of the threads.