11

I'm trying to make a simple logger class, and I want the ability to either log to either a generic ostream (cout/cerr) or a file. The design I have in mind is to allow the constructor to either take an ostream& or a filename, and in the latter case create an ofstream& and assign that to the class' private ostream& like so:

class Log {
private:
    std::ostream& os;
public:
    Log(std::ostream& os = std::cout): os(os) { }
    Log(std::string filename) {
        std::ofstream ofs(filename);
        if (!ofs.is_open())
            // do errorry things
        os = ofs;
    }
};

Doing such gives me an error that ofstream's assignment operator is private. Looking over that again, it occurred to me that making a reference to a local object probably wouldn't work, and making os a pointer to an ostream and declaring and deleting it on the heap worked with the ofstream case, though not with the ostream case, where the ostream already exists and is just being referenced by os (because the only place to delete os would be in the constructor, and I don't know of a way to determine whether or not os is pointing to an ofstream created on the heap or not).

So how can I make this work, i.e. make os reference an ofstream initialized with a filename in the constructor?

4 Answers4

11

For one thing, you can't rebind references once they're created, you can only initialise them. You might think you could do this:

Log(std::string filename) : os(std::ofstream(filename)) {
    if (!os.is_open())
        // do errorry things
}

But that's no good because you are making os refer to a temporary variable.

When you need a reference that has to be optional, that is, it needs to refer to something sometimes and not other times, what you really need is a pointer:

class Log {
private:
    std::ostream* os;
    bool dynamic;
public:
    Log(std::ostream& os = std::cout): os(&os), dynamic(false) { }
    Log(std::string filename) : dynamic(true) {
        std::ofstream* ofs = new std::ofstream(filename);

        if (!ofs->is_open())
            // do errorry things and deallocate ofs if necessary

        os = ofs;
    }

    ~Log() { if (dynamic) delete os; }
};

The above example is just to show you what is going on, but you probably will want to manage it with a smart pointer. As Ben Voigt points out, there are a lot of gotchas that will cause unforeseen and undesired behaviour in your program; for example, when you try to make a copy of the above class, it will hit the fan. Here is an example of the above using smart pointers:

class Log {
private:
    std::unique_ptr<std::ostream, std::function<void(std::ostream*)>> os;
public:
    Log(std::ostream& os = std::cout): os(&os, [](ostream*){}) { }

    Log(std::string filename) : os(new std::ofstream(filename), std::default_delete<std::ostream>()) {
        if (!dynamic_cast<std::ofstream&>(*os).is_open())
            // do errorry things and don't have to deallocate os
    }
};

The unusual os(&os, [](ostream*){}) makes the pointer point to the given ostream& but do nothing when it goes out of scope; it gives it a deleter function that does nothing. You can do this without lambdas too, it's just easier for this example.

Seth Carnegie
  • 73,875
  • 22
  • 181
  • 249
  • So, assuming I'm still creating the ofstream in the constructor, there really isn't any way to avoid declaring/deleting os on the heap, is there? –  Feb 27 '12 at 01:59
  • @Anachrome no there's no other way, because you want to create it in _one_ constructor (and not the other) but have it last after the constructor is exited. The only way to do that that I know of is to allocate it dynamically. – Seth Carnegie Feb 27 '12 at 02:01
  • Ouch! Careful with the destructor! `{ Log l; }` will try to call `delete &std::cout;`! You need a flag there. – R. Martinho Fernandes Feb 27 '12 at 02:03
  • A well. I'll manage. And in regards to your code, wouldn't there be problems deleting in the destructor if it was created via the first constructor (and therefore wasn't dynamically alocated)? Of course, this is all just more reason to use smart pointers, I guess. –  Feb 27 '12 at 02:04
  • @Anachrome yes, that is exactly right. That is also what R. Martinho Fernandes mentioned, and I have fixed it. – Seth Carnegie Feb 27 '12 at 02:05
  • Wouldn't it just be better to have an internal `ofstream` object and a `ostream*` pointing to either the internal `ofstream` or an external `ostream` so that you avoid the dynamic allocation *and* remove that ugly flag? – André Caron Feb 27 '12 at 02:26
  • @AndréCaron that's what Nawaz's answer did, but he deleted it for some reason. I had forgotten that `ofstream`s can be default constructed when I wrote this answer. – Seth Carnegie Feb 27 '12 at 02:27
  • 1
    You should mention why a smart pointer is needed. The example code is a complete disaster waiting to happen, since the compiler thinks its blitable and will generate a copy constructor that does. And that's just one of many potential gotchas – Ben Voigt Feb 27 '12 at 02:32
  • Unfortunately, the deleter type is an explicit template parameter of `unique_ptr`. This means you can't use it to store deleters of different types. You need some type-erasure, either with `std::function` or a `shared_ptr`. – R. Martinho Fernandes Feb 27 '12 at 03:06
  • Even better may be a simple deleter functor that also carries the flag, and acts accordingly. No need for elaborated type erasure and lambdas here. – Xeo Feb 27 '12 at 04:03
  • +1 if you can implement the copy constructor! Seriously, I need to be able to do `Log log(cerr); Log log2(log); log2.output("hey");` – Marc Eaddy Nov 15 '13 at 07:49
9

The simplest thing to do is just bind your reference to an ofstream, and make sure the ofstream lives as long as your object:

class Log
{
    std::ofstream byname;
    std::ostream& os;
public:
    Log(std::ostream& stream = std::cout) : byname(), os(stream) { }
    Log(std::string filename) : byname(filename), os(this->byname)
    {
        if (!os)
            // handle errors
    }
};

Exception safe, can't leak, and the compiler-generated special member functions are sane.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • There's a certain beauty in the simplicity of this that's actually making me think I might end up using this instead of the above answer. Not quite decided yet, though. –  Feb 28 '12 at 02:22
0

You have to initialize the os in the initialize list in the constructors, just as what you did in Log(std::ostream& os = std::cout): os_(os) { }, because os is a reference, which can not be assigned after initialized.

Qix - MONICA WAS MISTREATED
  • 14,451
  • 16
  • 82
  • 145
Ade YU
  • 2,292
  • 3
  • 18
  • 28
0

In my Log/Debug class I find it useful to create a static member variable:

class debug {
  public:
    ...

    // Overload operator() for printing values.
    template<class Type1>
      inline debug&
      operator()(const std::string& name1,
                 const Type1& value1)
      {
        // Prettify the name/value someway in another inline function.
        _stream << print_value(name1, value1) << std::endl;

        return *this;
      }

  private:
    ...
    static std::ostream& _stream;
};

And then in my debug.cc file:

std::ostream& debug::_stream = std::cerr;
nerozehl
  • 463
  • 1
  • 7
  • 19