Refactor: Logging Library

[thedevmystic]

Hello, @yanniknelson!!
Again, it's me!

I've noticed that logging library was not polished. So I've made it more functional.

As, you said you want to decouple the library and program code. So now we can,

#include <Log.h>

static auto piet_logger = Log::LogManager::Get("Piet");

// And do
LOG_INFO(piet_logger, "Piet Interpreter Started...")

LOG_ERROR(piet_logger, "Empty Image Provided.")

// ...

And in main.cpp:

int main() {
    Log::LogManager::Init(); // on startup

    // ... main code

    Log::LogManager::Shutdown(); // before termination
}

As you can see, it is quite decoupled.
It mirrors RCL library (Robotics Operating System Client Library).

The library now adheres to best practices. Like previously in LogManager.cpp, you were initializing the sinks in the loop, it can lead to race conditions. And is generally poor practice. In macros, defining them in one line leads to ancient devil known as "Poor Readable Code" JK, it's not a devil, but it do leads to poor readability :)

And if we want to replace spdlog, to say Google's logging library, we can easily do it as I've made it quite modular!

[yanniknelson]

might be work putting a check that calls IsInitialized if the initialised flag bool is unset. This would remove the need to initialize the logging manager in code that use this library

[thedevmystic]

I would strongly disagree with this.

As lifecycle should be symmetric, if we have a explicit Shutdown() which we should, then we should have explicit Init().

Here, Init has Initialization job, Get has creation, Shutdown has deinitialization, and drop has destruction.

As C++ philosophy goes,

Do one thing, and do it good.

We should not mix Initialization with creation. We should always prefer discipline over convience.

What do you say, @yanniknelson?

[yanniknelson]

I somewhat agree with you but not in entirety.

Most libraries won't be the code that calls init(), yet WILL expect it to have been called inorder for their logging to work.
This is not ideal, which is why i suggested calling the init in the get. A better pattern would be to separate the get and the logger creation with logger creation calling init if not already done and leaving logger creation and shutdown to the libraries that use them. then at the end of the program we call shutdownall for safety if we feel we need to.

thoughts?

[yanniknelson]

Honestly a better pattern still would be an RAII wrapper around logger references that shutdown the logger when they go out of scope, this would make managing the logger lifetimes implicit. (as long as they are member variables)

[thedevmystic]

RAII i didn't thought about that. Well reasoned!

[yanniknelson]

thanks 😁, i'm a big fan of RAII in the right places, though i've seen some AWFUL uses of it in my time.