Skip to content

Refactor Logger methods for clarity and const correctness #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 12, 2025
+47 −49
Merged

Refactor Logger methods for clarity and const correctness #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c07667
Refactor Logger methods for clarity and const correctness
kimkulling Nov 12, 2025
8b25a1a
Refactor Logger class comments and parameter types
kimkulling Nov 12, 2025
f438c5a
Change getDateTime to a const member function
kimkulling Nov 12, 2025
0358aef
Update Logger.cpp
kimkulling Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 39 additions & 39 deletions code/Common/Logger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,40 +30,41 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include <string>

namespace cppcore {

static void appendDomain(const String &domain, String &logMsg) {
if (!domain.isEmpty()) {
logMsg += '(';
logMsg += domain;
logMsg += ')';
}
}

static ::std::string stripFilename(const ::std::string &filename) {
if (filename.empty()) {
return filename;
namespace {
void appendDomain(const String &domain, String &logMsg) {
if (!domain.isEmpty()) {
logMsg += '(';
logMsg += domain;
logMsg += ')';
}
}

::std::string::size_type pos = filename.find_last_of("/");
if (pos == ::std::string::npos) {
return filename;

::std::string stripFilename(const ::std::string &filename) {
if (filename.empty()) {
return filename;
}

::std::string::size_type pos = filename.find_last_of("/");
if (pos == ::std::string::npos) {
return filename;
}
const ::std::string strippedName = filename.substr(pos + 1, filename.size() - pos - 1);

return strippedName;
}
const ::std::string strippedName = filename.substr(pos + 1, filename.size() - pos - 1);

return strippedName;
}

static void addTraceInfo(const String &file, int line, String &msg) {
if (Logger::getInstance()->getVerboseMode() == Logger::VerboseMode::Trace) {
msg += String(" (", 2);
std::string stripped = stripFilename(file.c_str());
msg += String(stripped.c_str(), stripped.size());
msg += String(", ", 2);
std::stringstream ss;
ss << line;
std::string lineno = ss.str();
msg += String(lineno.c_str(), lineno.size());
msg += ')';

void addTraceInfo(const String &file, int line, String &msg) {
if (Logger::getInstance()->getVerboseMode() == Logger::VerboseMode::Trace) {
msg += String(" (", 2);
std::string stripped = stripFilename(file.c_str());
msg += String(stripped.c_str(), stripped.size());
msg += String(", ", 2);
std::stringstream ss;
ss << line;
std::string lineno = ss.str();
msg += String(lineno.c_str(), lineno.size());
msg += ')';
}
}
}

Expand All @@ -82,7 +83,7 @@ bool AbstractLogStream::isActive() const {
Logger *Logger::sLogger = nullptr;

Logger *Logger::create() {
if (nullptr == sLogger) {
if (sLogger == nullptr) {
sLogger = new Logger;
}

Expand All @@ -98,7 +99,7 @@ void Logger::set(Logger *logger) {
}

Logger *Logger::getInstance() {
if (nullptr == sLogger) {
if (sLogger == nullptr) {
static_cast<void>(create());
}

Expand Down Expand Up @@ -189,8 +190,7 @@ void Logger::print(const String &msg, PrintMode mode) {
}
}

if (msg.size() > 8) {
if (msg[6] == '=' && msg[7] == '>') {
if (msg.size() > 8 && msg[6] == '=' && msg[7] == '>') {
mIntention += 2;
}
}
Expand Down Expand Up @@ -231,7 +231,7 @@ void Logger::registerLogStream(AbstractLogStream *pLogStream) {
mLogStreams.add(pLogStream);
}

void Logger::unregisterLogStream(AbstractLogStream *logStream) {
void Logger::unregisterLogStream(const AbstractLogStream *logStream) {
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify header file signature matches.

The const-correctness improvement is good, making the function more permissive by accepting both const and non-const pointers. However, since this is a public API change, ensure the header file declaration has been updated to match this signature.

Run the following script to verify the header file contains the matching signature:

🏁 Script executed:

#!/bin/bash
# Description: Verify that Logger.h declares unregisterLogStream with const parameter

# Search for the function declaration in header files
rg -n "unregisterLogStream" --type=h -C 2

Length of output: 372

Update header file signature to match implementation.

The header file at include/cppcore/Common/Logger.h:169 still declares the old signature without const, but the implementation adds const to the parameter. This signature mismatch will cause compilation errors.

Update line 169 from:

void unregisterLogStream(AbstractLogStream *pLogStream);

to:

void unregisterLogStream(const AbstractLogStream *pLogStream);

Also note the parameter name differs between header (pLogStream) and implementation (logStream)—consider standardizing for consistency.

🤖 Prompt for AI Agents 
In include/cppcore/Common/Logger.h around line 169, the declaration still uses a
non-const parameter but the implementation in code/Common/Logger.cpp at line 233
uses const; update the header declaration to match the implementation by
changing the parameter type to const AbstractLogStream * and, for consistency,
align the parameter name with the implementation (e.g., use logStream or
pLogStream consistently) so the signature and names match across header and
source.

if (nullptr == logStream) {
return;
}
Expand All @@ -254,8 +254,8 @@ Logger::~Logger() {
}
}

String Logger::getDateTime() {
static const uint32_t Space = 2;
String Logger::getDateTime() const {
static constexpr uint32_t Space = 2;
DateTime currentDateTime;
std::stringstream stream;
stream.fill('0');
Expand Down
18 changes: 8 additions & 10 deletions include/cppcore/Common/Logger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,11 @@ class DLL_CPPCORE_EXPORT AbstractLogStream {

//-------------------------------------------------------------------------------------------------
///
/// @brief This class implements a simple logger.
///
/// @brief This class implements a simple logger.
/// The logger is implemented as a singleton. You can attach several log streams to it, which can
/// be used to log the messages to several output channels like a window or a log file or something
/// else.
/// The granularity of the logged messages can be controlled by the severity of the logger. The
/// supported modes are normal ( no debug and info messages ), verbose ( all messages will be
/// be used to log the messages to several output channels like a window or a log file or something
/// else. The granularity of the logged messages can be controlled by the severity of the logger.
/// The supported modes are normal ( no debug and info messages ), verbose ( all messages will be
/// logged ) and debug ( the debug messages will be logged as well, be careful with this option ).
//-------------------------------------------------------------------------------------------------
class DLL_CPPCORE_EXPORT Logger final {
Expand All @@ -87,8 +85,8 @@ class DLL_CPPCORE_EXPORT Logger final {
enum class VerboseMode {
Invalid = -1, ///< Invalid marker
Normal, ///< Only warnings and errors will be logged.
Verbose, ///< Normal messages will be logged as well.
Debug, ///< All debug messages will be logged as well.
Verbose, ///< Normal messages will be logged as well.
Debug, ///< All debug messages will be logged as well.
Trace, ///< Will enable the tracing.
Count ///< Number of enums
};
Expand Down Expand Up @@ -166,12 +164,12 @@ class DLL_CPPCORE_EXPORT Logger final {

/// @brief Unregisters a registered log stream.
/// @param[in] pLogStream A pointer showing to the log stream.
void unregisterLogStream(AbstractLogStream *pLogStream);
void unregisterLogStream(const AbstractLogStream *pLogStream);

private:
Logger();
~Logger();
String getDateTime();
String getDateTime() const;

private:
// @brief The Standard log stream.
Expand Down