2

I need very simple logger to my application. But sometimes I don't want to use this logger to speed up my application.

My logger looks like:

class Logger
{
    public:
        Logger(bool isActive = true)
        {
            mIsActive = isActive;
            if (isActive)
            {
                out.open(pathToLogFile);
            }
        }

        static std::ofstream& log()
        {
            return out;
        }
    private:
        static bool mIsActive;
        static std::ofstream out;
};

In my application I use it as:

Logger(true);  // in one place
Logger::log() << "Log" << std::endl;

What if I don't want to use this logger?

Logger(false);  // in one place. it doesn't open any file.
Logger::log() << "Log" << std::endl; // it shouldn't write anywhere

What is behavior of << operator if I haven't opened file? Is it safe? Is it very fast? Is it good idea??

5gon12eder
  • 24,280
  • 5
  • 45
  • 92
peter55555
  • 1,413
  • 1
  • 19
  • 36
  • 8
    The thing that disturbs me most is how you abuse the constructor to manipulate `static` data members. – 5gon12eder Feb 04 '15 at 22:02
  • @5gon12eder that would be fun to do in languages without free functions. `class imAFunction {...}` ? – Quentin Feb 04 '15 at 22:04
  • Well, you could always evaluate the stream object in a boolean context to see if it is in a good state and only write if it is. Like so `if ( Logger::log() ) { Logger::log() << "blah blah"; }`. But anyway, without having checked the reference, yeah I think writing to a stream that isn't opened / in a good state should be safe. EDIT: Then again, maybe not: http://stackoverflow.com/questions/20693758/are-there-consequences-to-writing-to-an-unopened-stream – antred Feb 04 '15 at 22:09
  • @Quentin: I admit to having done that on several occasions to abuse template specializations: `template<> class imAFunction {int v; imAFunction(int e):v(e+2){} operator int() const {return v;} };` – Mooing Duck Feb 04 '15 at 22:28
  • @MooingDuck static functions inside specializable wrappers are overrated I guess. – Quentin Feb 04 '15 at 22:32

3 Answers3

2

There are a couple of issues with your logger.

If you use operator<< on the deactivated logger, nothing happens, but the status of the out stream is set to failure and it will keep it until you reset it. This means that if subsequently you'd reactiveate your logger, nothing would be written to it anymore.

THe same if you'd reactivate a logger while it's already active: the out.open() would also fail and nothing would be written anymore.

If you want to keep this design, you'll have to update your logger, and check in your constructor if out is already open (using out.is_open()), and if needed clear() the error flags.

Addendum: If you are worried about performance of all these << that would unnecessarily process hypothetical output when the logger is deactivated, then you could consider using a custom operator<< for your logger class, that would take into consideration mIsActive. This SO answer on another question shows how it could work.

Community
  • 1
  • 1
Christophe
  • 68,716
  • 7
  • 72
  • 138
  • @MooingDuck You're fully right in the case of `Logger(true)`. However in the case of `Logger(false)` it would be needed to close it the stream if it was previously opened; otherwise the logger might continue to write. – Christophe Feb 04 '15 at 22:36
0

Your approach has too much overhead in any case. Just use an ostream:

std::ostream logger(0);

In order to write to it, use inserters as usual, but you can also check if anything is connected at all to avoid the overhead:

if(logger)
    logger << "some output" << endl;

Now, in order to direct the output somewhere, you attach a streambuffer:

logger.rdbuf(cout.rdbuf()); // redirect to stdout

You can also use a file as target:

logger.rdbuf(new filebuf("log.text", ios_base::out)); // redirect to a file

Disclaimer: I haven't looked up the exact parameters for the filebuf creation, but still hope you get the idea.

Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
0

I have run a very simple benchmark:

void
do_log(std::ostream& os)
{
  for (long i = 0L; i < 100000000L; ++i)
    os << i << "\n";
}

Passing it a default-constructed std::ofstream (not connected to any file)

std::ofstream sink {};
do_log(sink);

makes the program (compiled using GCC 4.9 with -O3 optimization level) take 3.1 seconds to complete. If instead, I connect the output to /dev/null

std::ofstream sink {"/dev/null"};
do_log(sink);

it takes 14.2 seconds, more than four times longer. So, yes, writing to an invalid stream seems to be quite efficient.

Note however, that the above code differs from your example. If I write it as

void
do_log(std::ostream& os)
{
  for (long i = 0L; i < 100000000L; ++i)
    os << i << std::endl;
}

instead, it takes 3.7 seconds if the stream is disconnected (which makes sense because anything written to it is discarded anyway) but 35.5 seconds if connected to /dev/null. Therefore, if you are concerned about performance, the first thing you should probably get rid of is the repetitive output flushing if you don't absolutely need it.

5gon12eder
  • 24,280
  • 5
  • 45
  • 92