8
\$\begingroup\$

I'm new to C++, and I wanted to add a way to log in a fancy way. As such, I created a small bit of code to do so, but I'm wondering how I can improve my code, especially the fact that DebugColors consists of two files, and regarding my use of templates, as I don't think the adder method(s) of the Debug struct is the best way to concatenate strings.

Additionally, I'm unsure if I should be using a struct in order to contain the methods, or if I should use a namespace for it, as I only want to be able to access the public methods from outside the struct.

DebugColors.hpp

#ifndef DEBUG_COLORS
#define DEBUG_COLORS

#include <string>

namespace DebugColors {
    extern std::string reset;
    extern std::string red;
    extern std::string green;
    extern std::string yellow;
    extern std::string blue;
}

#endif

DebugColors.cpp

#include "DebugColors.hpp"

namespace DebugColors {
    std::string reset = "\x1b[0m";
    std::string red = "\x1b[31m";
    std::string green = "\x1b[32m";
    std::string yellow = "\x1b[33m";
    std::string blue = "\x1b[34m";
}

Debug.hpp

#ifndef DEBUG_HPP
#define DEBUG_HPP

#include <string>
#include <iostream>
#include "DebugColors.hpp"

struct Debug {
    private:
        template<typename T>
        static T adder(T v) {
            return v;
        }

        template<typename T, typename ...Args>
        static T adder(T first, Args ...args) {
            return first + " " + adder<std::string>(args...);
        }

        template<typename ...T>
        static void LogMessage(T&... args) {
            std::string message = adder<std::string>(args...);
            
            std::cout << message << std::endl;
        }
    public:
    template<typename ...T>
        static void Log(T&... args) {
            LogMessage(args...);
        }

        template<typename ...T>
        static void Info(T&... args) {
            LogMessage(DebugColors::blue, "[INFO]", args..., DebugColors::reset);
        }

        template<typename ...T>
        static void Warning(T&... args) {
            LogMessage(DebugColors::yellow, "[WARN]", args..., DebugColors::reset);
        }
        
        template<typename ...T>
        static void Error(T&... args) {
            LogMessage(DebugColors::red, "[ERROR]", args..., DebugColors::reset);
        }

        template<typename ...T>
        static void Success(T&... args) {
            LogMessage(DebugColors::green, "[SUCCESS]", args..., DebugColors::reset);
        }
};

#endif
```
\$\endgroup\$
4
\$\begingroup\$

Using namespaces

We can put everything into a namespace which would enable us to use using.

Consider this example,

using Debug::Log; //Error if it's a static member function
                  // Error msg: error: ‘Debug’ is not a namespace or unscoped enum

namespace Debug{
   Type Func(){ ... };
}

using Debug::Func; // No error

Func(); // Calls `Debug::Func`

Since, Log, Info, Error, Warning and Success are just standalone utility functions. I see no reason to put them in a class.

We can put private functions into a different namespace say impl, a namespace which would contain private functions of Debug class.

namespace impl{
  std::string add(...) {...}
  void LogMessage(...) {...}
}

namespace Debug{
  void Log(...) { impl::LogMessage(...) }
  void Info(...) { impl::LogMessage(...) }
  void Error(...) { impl::LogMessage(...) }
}

Use std::to_string to print ints and floats

Create a to_str function which would call std::string. Why this is necessary is explained in the below paragraphs.

namespace impl {

template<typename T>
std::string to_str(const T& val){
    return std::to_string(val);
}

std::string to_str(const std::string& val){
    return val;
}

std::string to_str(const char * str){
    return std::string(str);
}

} // namespace impl

Fold Expression since C++17

We can leverage fold expression when working with variadic functions. We can rewrite the adder as such.

namespace impl{

template <typename... Args>
std::string adder(Args... args) {
  return ((to_str(args) + " ") + ...);
}

}

forwarding reference

Your current implementation we can only use lvalues with functions. Using forwarding reference we can pass both lvalues and rvalues.

namespace Debug{

template <typename... T>
void Log(T&&... args) {
  impl::LogMessage(args...);
}
// Similarly for other functions too.
}

Full code

Debug.hpp

#ifndef DEBUG_HPP
#define DEBUG_HPP

#include <iostream>
#include <string>

#include "DebugColors.h"

namespace impl {

template<typename T>
std::string to_str(const T& val){
    return std::to_string(val);
}

std::string to_str(const std::string& val){
    return val;
}

std::string to_str(const char * str){
    return std::string(str);
}

template <typename... Args>
std::string adder(Args&&... args) {
  return ((to_str(args) + " ") + ...);
}

template <typename... T>
void LogMessage(T&&... args) {
  std::string message = adder(args...);

  std::cout << message << std::endl;
}
}  // namespace impl

namespace Debug {
template <typename... T>
void Log(T&&... args) {
  impl::LogMessage(args...);
}

template <typename... T>
void Info(T&&... args) {
  impl::LogMessage(DebugColors::blue, "[INFO]", args..., DebugColors::reset);
}

template <typename... T>
void Warning(T&&... args) {
  impl::LogMessage(DebugColors::yellow, "[WARN]", args..., DebugColors::reset);
}

template <typename... T>
void Error(T&&... args) {
  impl::LogMessage(DebugColors::red, "[ERROR]", args..., DebugColors::reset);
}

template <typename... T>
void Success(T&&... args) {
  impl::LogMessage(DebugColors::green, "[SUCCESS]", args...,
                   DebugColors::reset);
}

}  // namespace Debug

#endif

test.cpp

include "Debug.hpp"

using Debug::Error;
int main(){
  int t = 10;
  Error("Danger", 20, t, 3.14);
}

Output:

enter image description here


You can use clang-format for formatting your code files.

Terminal command:

clang-format -style=Google -i files
\$\endgroup\$
5
  • \$\begingroup\$ Good review! What do you think about also mentioning std::string_view for the color codes? \$\endgroup\$
    – Edward
    Sep 3 at 13:49
  • \$\begingroup\$ @Edward I would suggest that too. But this rewritten code would unnecessarily convert it to string each time. It also converts const char*. \$\endgroup\$
    – JDługosz
    Sep 3 at 16:33
  • \$\begingroup\$ @Ch3steR instead of to_str call it wrap or something that does not imply a specific return type. Use string_view as returns for const char* and string_view itself, as well as string so you don't have to copy it. But keep string return for numbers and such where it allocates memory as part of the conversion. \$\endgroup\$
    – JDługosz
    Sep 3 at 16:35
  • \$\begingroup\$ @Edward Thank you. Yes string_view is much better. \$\endgroup\$
    – Ch3steR
    Sep 3 at 17:37
  • 1
    \$\begingroup\$ @JDługosz Yes agreed, to_str is unnecessarily creating copies. I'm not near my laptop. Can you add it an answer since it's very good suggestion? \$\endgroup\$
    – Ch3steR
    Sep 3 at 17:39
3
\$\begingroup\$

Everything @Ch3steR said

Const Correctness

You don't want some other developer breaking your code by accidentally changing the value of your color variables. So you should make them costant (that will make sure the compiler does not allow any alterations:

namespace DebugColors {
    extern std::string const reset;
    extern std::string const red;
    extern std::string const green;
    extern std::string const yellow;
    extern std::string const blue;
}

You can also use std::string literals (rather than C-String literals that call the constructor).

namespace DebugColors {

    using namespace std::string_literals;

    std::string const reset  = "\x1b[0m"s;   // Notice the s
    std::string const red    = "\x1b[31m"s;
    std::string const green  = "\x1b[32m"s;
    std::string const yellow = "\x1b[33m"s;
    std::string const blue   = "\x1b[34m"s;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.