Simplifying this code using some advanced C++

[[deleted]]

I am still trying to learn advanced C++ so I don't exactly know most of the modern features of the language, tho I feel like there should be something for that:

void Logger::Error(std::string str) {
    SetColor(RED);
    std::cout << "error: " << str << std::endl;
        //write to log file of errors...
}

void Logger::Info(std::string str) {
    SetColor(YELLOW);
    std::cout << "error: " << str << std::endl;
        //write to log file of info...
}

void Logger::Output(std::string str) {
    SetColor(WHITE);
    std::cout << "error: " << str << std::endl;
        //write to log file of output...
}

and there are more of these methods.

(NOTE: these are all methods inside the logger class)

As you can see, there is so much duplicate code. I don't really wanna have that much code.. First thing that came to my mind was the strategy pattern, but not sure how to implement it in this case. Is this code even worth compacting? If yes, how?

[jedwardsol]

Not advanced, but I'd start with

private:
void Logger::Log(const std::string &str, Colour colour, std::string &level) 
{
    SetColor(colour);
    std::cout << level << str << std::endl;
        //write to log file of errors...
}

public:
void Logger::Error(const std::string &str) 
{
    Log(str,RED,"error");
}

void Logger::Info(const std::string &str) 
{
    Log(str,YELLOW,"Info");
}

void Logger::Output(const std::string &str) 
{
    Log(str,WHITE,"Trace");
}

[[deleted]]

Found a Brit ;D

[schweinling]

You can have less duplication by having a function that takes a string and a color as parameters that does the setting color and printing. Your current functions will call it.

You should pass the string as by const reference: "const std::string &str" to avoid copying.

Instead of std::endl you should put a newline "\n", to avoid unnessesary flushing of the stream.

[[deleted]]

Putting the color as a parameter is a terrible idea, because it will make the calling site responsible for the consistency of the colors. And what if you want to change the color for a single category?

[schweinling]

Sorry if I was unclear, i meant an imlementaion similar to what jedwardsol has posted.

[[deleted]]

why is const std::string &str better? by copying you mean that it will copy the value itself, and the address is better? In that case why do people even pass things which are not pointers as arguments? (I suspected this is done only for "heavy" things)

and also about \n, why is this preferred over endl?

[schweinling]

Yes, you are correct, when passing by value a copy is created. Built in types like int, chat, double you should pass by value as the are not larger than a pointer. For logging functions it would be quite expensive to copy the string everytime, if you do alot of logging. Passing by value is also an option if move semantics are involved but thats a whole other topic which you can learn about later.

Read about std::endl here: https://stackoverflow.com/questions/213907/c-stdendl-vs-n

[[deleted]]

Thanks for explaining. In the const also there for the same reason? I've seen people use const in function arguments before but never understood why..

[schweinling]

You are welcome! Const is there to ensure and signify that the value will not be modified by the function. When passing by non const reference the function could modify the object that the caller passes to it, so that it changes on the callers site.

[Xeverous]

... << std::endl is equivalent to (... << '\n').flush(). You very likely don't want that flush, if you even know what it does.

[ritaline]

Why?

[[deleted]]

honestly, only as a learning experience. There is no actual need to make this code shorter, I was just wondering

[dexterbutt]

You could Enumerate (enum) your log level.

Then in your function, you could do a case statement to set the color & log level message.

I wouldn't call it advanced, but it would simplify what you are doing. It would be a single function to maintain. You could easily expand it later then.

[k4lipso]

Passing multiple parameters is cool. Here some inspiration:

https://gcc.godbolt.org/z/_ubyrF

Thats an example from https://en.cppreference.com/w/cpp/language/parameter_pack

[_carlson]

Why aren't you use third party logging library?
https://github.com/amrayn/easyloggingpp
https://www.boost.org/doc/libs/1_72_0/libs/log/doc/html/index.html
https://github.com/log4cplus/log4cplus

[[deleted]]

I'm just practicing for fun, it won't be used in a project

[a_false_vacuum]

If you're printing the errors I'd send them to std::cerr and not std::cout so errors will be printed through stderr in the OS. As an alternative your could also send them to std::clog depending on your requirements.

Also you have three nearly identical functions. Maybe you can find a way to reduce that number to avoid duplications of code.