Best practice when dealing with C++ iostreams

user_10899376 0
Viewed 105

I'm writing a command-line utility for some text processing. I need a helper function (or two) that does the following:

  1. If the filename is -, return standard input/output;
  2. Otherwise, create and open a file, check for error, and return it.

And here comes my question: what is the best practice to design/implement such a function? What should it look like?

I first considered the old-school FILE*:

FILE *open_for_read(const char *filename)
{
    if (strcmp(filename, "-") == 0)
    {
        return stdin;
    }
    else
    {
        auto fp = fopen(filename, "r");
        if (fp == NULL)
        {
            throw runtime_error(filename);
        }
        return fp;
    }
}

It works, and it's safe to fclose(stdin) later on (in case one doesn't forget to), but then I would lose access to the stream methods such as std::getline.

So I figure, the modern C++ way would be to use smart pointers with streams. At first, I tried

unique_ptr<istream> open_for_read(const string& filename);

This works for ifstream but not for cin, because you can't delete cin. So I have to supply a custom deleter (that does nothing) for the cin case. But suddenly, it fails to compile, because apparently, when supplied a custom deleter, the unique_ptr becomes a different type.

Eventually, after many tweaks and searches on StackOverflow, this is the best I can come up with:

unique_ptr<istream, void (*)(istream *)> open_for_read(const string &filename)
{
    if (filename == "-")
    {
        return {static_cast<istream *>(&cin), [](istream *) {}};
    }
    else
    {
        unique_ptr<istream, void (*)(istream *)> pifs{new ifstream(filename), [](istream *is)
                                                      {
                                                          delete static_cast<ifstream *>(is);
                                                      }};
        if (!pifs->good())
        {
            throw runtime_error(filename);
        }
        return pifs;
    }
}

It is type-safe and memory-safe (or at least I believe so; do correct me if I'm wrong), but this looks kind of ugly and boilerplate, and above all, it is such a headache to just get it to compile.

Am I doing it wrong and missing something here? There's gotta be a better way.

iostream smart-pointers c++
3 Answers

I would probably make it into

std::istream& open_for_read(std::ifstream& ifs, const std::string& filename) {
    return filename == "-" ? std::cin : (ifs.open(filename), ifs);
}

and then supply an ifstream to the function.

std::ifstream ifs;
auto& is = open_for_read(ifs, the_filename);

// now use `is` everywhere:
if(!is) { /* error */ }

while(std::getline(is, line)) {
    // ...
}

ifs will, if it was opened, be closed when it goes out of scope as usual.

A throwing version might look like this:

std::istream& open_for_read(std::ifstream& ifs, const std::string& filename) {
    if(filename == "-") return std::cin;
    ifs.open(filename);
    if(!ifs) throw std::runtime_error(filename + ": " + std::strerror(errno));
    return ifs;
}
user_75822470

As an alternative to Ted's answer (which I think I prefer, actually), you could make your custom deleter a bit smarter:

auto stream_deleter = [] (std::istream *stream) { if (stream != &std::cin) delete stream; };
using stream_ptr = std::unique_ptr <std::istream, decltype (stream_deleter)>;

stream_ptr open_for_read (const std::string& filename)
{
    if (filename == "-")
        return stream_ptr (&std::cin, stream_deleter);

    auto sp = stream_ptr (new std::ifstream (filename), stream_deleter);
    if (!sp->good ())
        throw std::runtime_error (filename);
    return sp;
}

Then the same deleter works for both cases and there are no typing problems.

Live demo

user_57432880

Something which I've used in the past was calling rdbuf to change the buffer of std::cin. That may be useful if you don't want to change existing code using std::cin. You have to pay attention not to use the buffer after it has been destroyed, but, that's nothing that a RAII wrapper can't solve. Something like (not tested, not even proven correct):

struct stream_redirector {
    stream_redirector(std::iostream& s, std::string const& filename,
                      std::ios_base::openmode mode = ios_base::in) 
       : redirected_stream_{s}
    {
        if (filename != "-") {
           stream_.open(filename, mode);
           if (stream_) {
               throw std::runtime_error(filename + ": " + std::strerror(errno));
           saved_buf_ = redirected_stream_.rdbuf();
           redirected_stream_.rdbuf(stream_.rdbuf());
        }
    }
    ~stream_redirector() {
        if (saved_buf_ != nullptr) {
           redirected_stream_.rdbuf(saved_buf_);
        }
    }
private:
    std::stream& redirected_stream_;
    std::streambuf* saved_buf_{nullptr};
    std::fstream stream_;
};

To be used:

    ...
    stream_redirector cin_redirector(std::cin, filename);
    std::string str;
    std::cin >> str;
    ...
user_1362080
Related