Logger with a Core and Client side

Question

This logger is for a game engine I'm writing it has a core side which is the game itself and a client side which is the engine. The architecture of the code I feel like is messy it has the main log files which is a .h and a cpp and each client and core has its own .h and cpp file. Is there any way I can improve this logger?

LOG.H

#pragma once
#include <iostream>
#include <mutex>
#include <chrono>
#include <ctime>
#include <iomanip>
#include <sstream>
#include <string>
enum class LogLevel { TRACE, DEBUG, INFO, WARN, ERROR, CRITICAL };
class Logger {
public:
 explicit Logger(LogLevel level = LogLevel::INFO);
 virtual void log(LogLevel level, const std::string& message);
 void setLogLevel(LogLevel level);
protected:
 std::mutex mutex;
 LogLevel logLevel;
 std::string getTimestamp();
 virtual std::string logLevelToString(LogLevel level);
 virtual std::string formatMessage(LogLevel level, const std::string& message);
 virtual void outputLog(const std::string& formattedMessage);
 std::string getColorCode(LogLevel level);
};
#define LOG_TRACE(logger, msg) (logger).log(LogLevel::TRACE, "Walnut: " + std::string(msg))
#define LOG_DEBUG(logger, msg) (logger).log(LogLevel::DEBUG, "Walnut: " + std::string(msg))
#define LOG_INFO(logger, msg) (logger).log(LogLevel::INFO, "Walnut: " + std::string(msg))
#define LOG_WARN(logger, msg) (logger).log(LogLevel::WARN, "Walnut: " + std::string(msg))
#define LOG_ERROR(logger, msg) (logger).log(LogLevel::ERROR, "Walnut: " + std::string(msg))
#define LOG_CRITICAL(logger, msg) (logger).log(LogLevel::CRITICAL, "Walnut: " + std::string(msg))
#define LOG_CORE_TRACE(logger, msg) (logger).log(LogLevel::TRACE, "Walnut: " + std::string(msg))
#define LOG_CORE_DEBUG(logger, msg) (logger).log(LogLevel::DEBUG, "Walnut: " + std::string(msg))
#define LOG_CORE_INFO(logger, msg) (logger).log(LogLevel::INFO, "Walnut: " + std::string(msg))
#define LOG_CORE_WARN(logger, msg) (logger).log(LogLevel::WARN, "Walnut: " + std::string(msg))
#define LOG_CORE_ERROR(logger, msg) (logger).log(LogLevel::ERROR, "Walnut: " + std::string(msg))
#define LOG_CORE_CRITICAL(logger, msg) (logger).log(LogLevel::CRITICAL, "Walnut: " + std::string(msg))

LOG.CPP

#include "Log.h"
Logger::Logger(LogLevel level) : logLevel(level) {}
void Logger::log(LogLevel level, const std::string& message) {
 if (static_cast<int>(level) < static_cast<int>(logLevel)) {
 return;
 }
 std::lock_guard<std::mutex> lock(mutex);
 auto timestamp = getTimestamp();
 auto formattedMessage = formatMessage(level, message);
 outputLog(getColorCode(level) + "[" + timestamp + "] " + formattedMessage);
}
void Logger::setLogLevel(LogLevel level) {
 logLevel = level;
}
std::string Logger::getTimestamp() {
 auto now = std::chrono::system_clock::now();
 auto now_c = std::chrono::system_clock::to_time_t(now);
 std::stringstream ss;
 ss << std::put_time(std::localtime(&now_c), "%F %T");
 return ss.str();
}
std::string Logger::logLevelToString(LogLevel level) {
 // Implement log level to string conversion if needed
 return "";
}
std::string Logger::formatMessage(LogLevel level, const std::string& message) {
 // Modify as needed based on the required format
 return logLevelToString(level) + "" + message;
}
void Logger::outputLog(const std::string& formattedMessage) {
 std::cout << formattedMessage << "\x1b[0m" << std::endl << std::flush; // Reset color
}
std::string Logger::getColorCode(LogLevel level) {
 switch (level) {
 case LogLevel::TRACE:
 return "\x1b[90m"; // Grey
 case LogLevel::DEBUG:
 return "\x1b[34m"; // Blue
 case LogLevel::INFO:
 return "\x1b[32m"; // Green
 case LogLevel::WARN:
 return "\x1b[93m"; // Yellow
 case LogLevel::ERROR:
 return "\x1b[38;2;255;165;0m"; // Orange
 case LogLevel::CRITICAL:
 return "\x1b[31m"; // Red
 default:
 return "";
 }
}

Main.cpp

#include <iostream>
#include "CoreLogger.h"
#include "ClientLogger.h"
int main() {
 Logger logger(LogLevel::TRACE);
 while (true) {
 // Logging using Client identifier
 LOG_TRACE(logger, "This is a trace message from Client.");
 LOG_DEBUG(logger, "This is a debug message from Client.");
 LOG_INFO(logger, "This is an info message from Client.");
 LOG_WARN(logger, "This is a warning message from Client.");
 LOG_ERROR(logger, "This is an error message from Client.");
 LOG_CRITICAL(logger, "This is a critical message from Client.");
 // Logging using Core identifier
 LOG_CORE_TRACE(logger, "This is a trace message from Core.");
 LOG_CORE_DEBUG(logger, "This is a debug message from Core.");
 LOG_CORE_INFO(logger, "This is an info message from Core.");
 LOG_CORE_WARN(logger, "This is a warning message from Core.");
 LOG_CORE_ERROR(logger, "This is an error message from Core.");
 LOG_CORE_CRITICAL(logger, "This is a critical message from Core.");
 }
 return 0;
}

• Ajinkya Kamat: why are you marking log() and a few methods in Logger class as virtual. Do you every expect someone to override those and have a custom implementation? I would argue that in a logger class there is no need for that. But would really like to understand your requirements before I review the code. A good way to think about it to write a quick an dirty client code for the use case. You already know how some core funcationality code might seem good till you actually write client side (refer to "The architecture of the code I feel like is messy") .
• rrandel: the client and core loggers are overriding the virtual classes its what i found worked easiest. I guess I could've just used something like spdlog but I wanted to get some experience writing my own. its gonna be for a game engine so i guess it should be memory efficient and quick I dont need any file logging right now its just console logging
• Ajinkya Kamat: What do you mean by client and core? Do you have some code for that? Or atleast some description?

Answer 1

This assumes that you are using C++17 based on previous discussions.

1. Prefer composition over inheretance. Here, you can create a single non-virtual logger class. Then create an interface for IO and a formatter classes. That way you dont have to override all the methods if you want to customize a logger. It also seperate concerns by confining IO operations to IO class and formatting operations to format class. Checkout spdlog's sink class. It is a good example.
2. You dont need the mutex in logger class if you seperate the concerns like i described. This way non threaded loggers will not have to pay for the mutex. Makes it easier to reason too.
3. For functions returning hardcoded strings like logLevelToString() and getColorCode(). Use std::string_view. std::string is expensive and involves memory allocation followed by a copy operation. You already have the string literal in memory so there is no point creating a string.
4. logLevel member in Logger has a bit of a confusing name call it maxLogLevel
5. If you are going to arithematic with enum classes define a underlying type and then use std::underlying_type_t to retrive it. Dont just cast to int. Doing the former may lead to undefined behavior. It is rare since int on most platform is very large.
6. std::flush is redudant
7. LOG_CORE_* macros are redudant remove them.
8. Use __LINE__, __FILE__ to log the filename and line number. Without that a log is much more difficult to use.
9. Provide a streaming interface for people who like that.