2

I'm trying to create a simple logger which logs different messages to disk. The logger is a simple class of which I've overridden the << operator and, in order to save the messages, I send an enum value signaling to save what has come so far. basically flush everything to the disk.

This is how I intend on using this logger:

Logging::Logger << "First " << 255 << "Second" << exc.what() << Logging::Save; 

As you can see, since I need a way to distinuish between controling arguments such as Logging::Save and other normal arguments, I need to know what I'm dealing with. For getting the type of argument sent, I used std::is_same. However, it seems for some reasons the check doesn't work and I get the following error :

 In instantiation of 'Logging::Log& Logging::Log::operator<<(const T&) [with T = char [19]]': 104:35: required from here 47:31: error: no match for 'operator==' (operand types are 'const char [19]' and 'Logging::Mode') In instantiation of 'Logging::Log& Logging::Log::operator<<(const T&) [with T = Logging::Mode]': 104:67: required from here 57:28: error: cannot bind 'std::basic_ostream<char>' lvalue to 'std::basic_ostream<char>&&' In file included from /usr/include/c++/4.9/iostream:39:0, from 10: /usr/include/c++/4.9/ostream:602:5: note: initializing argument 1 of 'std::basic_ostream<_CharT, _Traits>& std::operator<<(std::basic_ostream<_CharT, _Traits>&&, const _Tp&) [with _CharT = char; _Traits = std::char_traits<char>; _Tp = Logging::Mode]' operator<<(basic_ostream<_CharT, _Traits>&& __os, const _Tp& __x) ^ 

This is what I have come up so far :

#include <iostream> #include <fstream> #include <sstream> #include <typeinfo> #include <type_traits> namespace Logging { enum class Mode { Save = 0, Load = 1 }; static constexpr Mode Save = Mode::Save; class Log { private: std::stringstream stream; bool isNew = true; bool displayResults = false; public: template <typename T> Log& operator<<(const T& value) { if (isNew) { stream << "start : "; isNew = false; } if (std::is_same<T, Logging::Mode>::value) { if (value == Mode::Save) Save(); if (displayResults) std::cout << std::endl; isNew = true; } else { stream << value; } if (displayResults) std::cout << stream.str(); return *this; } void Save(std::string logFilename= "Log.txt") { isNew = true; std::ofstream logFile; logFilename = (logFilename == "") ? "Log.txt" : logFilename; logFile.open(logFilename, std::ios::out | std::ios::app); logFile << this->Get(); this->stream.clear(); this->stream.str(""); } Log& Display(bool status) { displayResults = status; return *this; } std::string Get() const { return this->stream.str(); } friend std::ostream& operator<<(std::ostream& os, const Log& log); }; std::ostream& operator<<(std::ostream& os, const Log& log) { os << log.Get(); return os; } Log Logger; } int main() { Logging::Logger.Display(true)<< "What is your name?" <<Logging::Save; } 

What am I missing here?

2
  • 1
    Have you considered an overload of operator<< for a Logging::Mode argument? Commented Sep 29, 2020 at 9:31
  • @Botje No, should'nt the default one be able to handle that knowing that we are using a template ? Commented Sep 29, 2020 at 9:32

1 Answer 1

3

You have it almost correct, if constexpr(std::is_same<T, Logging::Mode>::value) is what you want in C++17.

Without constexpr the body of the if statement is compiled no matter the condition, this did generate the error about comparing arrays with enums even though the branch is never taken in such case.

'operator==' (operand types are 'const char [19]' and 'Logging::Mode').

Consider using universal references template<typename T> T&& and perfect forwarding for passing value around. It is good practice.

In your case a simple overloaded helper method works too:

template <typename T> Log& operator<<(T&& value) { if (isNew) { stream << "start : "; isNew = false; } helper(std::forward<T>(value)); if (displayResults) std::cout << stream.str(); return *this; } template<typename T> void helper(T&& value){ stream << std::forward<T>(value); } // Perfect forwarding requires that the enum is taken by value, // otherwise the template is a better match in some cases. // - Or you would have to write both const& and && versions. void helper(Logging::Mode value){ if (value == Mode::Save) Save(); if (displayResults) std::cout << std::endl; isNew = true; } 
Sign up to request clarification or add additional context in comments.

3 Comments

std::forward instead of std::move as you use perfect forwarding. but I would prefer simply const T& in that case (no different output/case if passed argument is a const l-value, a non-const lvalue, a rvalue).
@Jarod42 Thanks, fixed it. Well, as I said, I think it is good practice. you do not known which type the user puts in and they are also in the control of operator<<(std::ostream,T). Maybe they can leverage the nature of the value passed to it.
In that case (operator <<), your "good practices" make overloads (and if constexpr version too) more complicated (not a big deal with enum or small/trivial types, but for more "complicated" types such as std::string or std::vector). It is IMO opinion based. Here it is simply more verbose, more complex for no real benefits.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.