Accept either a existing stream or a filename (to be opened) in a constructor Context:

In a larger project, I am trying to build an utility class to encapsulate the parsing of input data. And I want it to be able to process either an already existing input stream, or a file given by its name.

The general idea is to build a stream converter. The input file contains a variable number of (possibly multi-line records). The program parses the file, retrieves one item, convert it and write it to the output file. So the structure of the program (pseudo code here) is:

Parser parser(input_stream);
Writer writer(output_stream);

while (StopRecord != (record = parser.getRecord())) {
    writer.write(record);
}


My first idea was to have a ref to a std::istream as a class member, and initialize it in constructor, either from an existing ref or to a newly opened file

class Parser {
    std::istream &amp;in;
public:
    Parser(std::istream&amp; in): in(in) {}   // fine
    Parser(std::string file): in(std::ifstream(file)) {}   // compile error

    Record getRecord();
};


The problem is that std::ifstream(file) is a temporary, and can only initialize a const lvalue reference or a rvalue reference. A const istream would be pretty useless, so I tried a rvalue ref.

class Parser {
    std::istream &amp;&amp;in;
public:
    Parser(std::istream&amp; in): in(in) {}   // compile error
    Parser(std::string file): in(std::ifstream(file)) {}   // fine

    Record getRecord();
};


But now a std::istream&amp; cannot be used to initialize a std::istream&amp;&amp;, and I would not dare to use Parser(std::istream&amp;&amp; in): in(in) {} and later p = Parser(std::cin); with a move from a standard stream!

Current code:

I finally decided to dynamically allocate a std::ifstream in my class when I have to process a file to have an lvalue to initialize my ref:

class Parser {
    std::ifstream *fin;
    std::istream&amp; in;   // MUST be after fin declaration!
public:
    Parser(std::istream&amp; in): fin(nullptr), in(in) {}
    Parser(std::string file): fin(new std::ifstream(file)), in(*fin) {}
    ~Parser() {
        if (fin) {
            fin.close();
            delete fin;
        }
    }

    Record getRecord();
};


It seems to work (even if in case of copies all will share the same underlying stream), but requires fin to be declared before in, and the raw pointer initialization looks rather ugly.

Question:

As this design come from my former C experience, I wonder whether this follows modern C++ best practices and how I could make it better, more robust and easier to maintain
Very close

You're actually one foot off the finish.

Reduce

You could store a pointer to the istream (note that std::ifstream is its child class, so new std::ifstream(file) will automatically be converted to istream*), then store a bool flag if it is owning or not.

Bigger issue

There is yet greater issue here, and it is ownership semantics. When ownership semantics are weird design starts becoming fragile. I'd recommend letting the caller to take ownership of the stream, so that Parser wouldn't mess with it and keep rule of zero. Callers will have much more power that way. Otherwise Parser would also require implementing either of rule of 0/3/5. 

Code review

fin.close();


is redundant in the destructor. Invoking it explicitly opens possibilities for exceptions, thus making your fin leak. Just delete it, destructor will take care of closing.

Code never checks if the file is opened. Code could start on non-opened stream, which is, I believe undefined behavior.