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.

CodePudding user response：

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;
}

CodePudding user response：

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

CodePudding user response：

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;
    ...