How do I access a pointer's subclass's methods if those methods are unique to the subclass?

Question

A part of my program has two possible cases: (1) if the user only gives 2 command line arguments, take input from standard input (cin) (2) if the user gives 3 command line arguments (the last one being a file name), take input from the file. In order to not be reusing the same code for for both options, I tried to use a pointer to a superclass of both cin and ifstream, istream for both means of input.

My issue is that on lines 5 and 22 of the code below, I'm trying to reference methods only available to the subclass ifstream (open and close). Based on my logic, if those methods are called the pointer has to be pointing to a type ifstream, but the program fails to compile because those methods aren't defined in the istream class.

Is there any way to get around this?

istream *currentStream;
    if (argc == 3) {
        // Handle optional file input
        currentStream = new ifstream(argv[2]);
        currentStream->open(argv[2]);
        if (currentStream->fail()) {
            cerr << "FILE COULD NOT BE OPENED\n";
            return 1;
        }
    } else {
        currentStream = &cin;
    }
    string myLine;
    // go line by line and translate it
    while (getline(*currentStream, myLine)) {
        if (currentStream->eof()) {
            break;
        }
        cout << rot13(myLine) << endl;
    }
    if (dynamic_cast<ifstream*>(currentStream)) {
        currentStream->close();
    }
    // handle pointer
    delete currentStream;
    currentStream = NULL;
    return 0;

You probably would want to wrap those in separate classes, where you could call `open` and `close`, and the class without the `ifstream` would just do nothing, and the other class would actually open and close the `ifstream`. You have the right idea with `dynamic_cast` though, but you should _get_ the return like `if (ifstream* filestream = dynamic_cast...) { filestream->close()` — Tas, Sep 25 '19 at 21:52
I started writing the answer, but then I realized, that those lines, that are causing your issues: are not necessary. If you pass value to the constructor of `std::ifstream` - it will call `open` on its own. And call to `close` is unnecessary, because the destructor, of `std::ifstream`, will call it. That leaves with the one, true, problem: `delete currentStream;` is undefined behavior, when `argc != 3`, due to it trying to `delete &cin`. — Algirdas Preidžius, Sep 25 '19 at 21:57

score 3 · Accepted Answer · answered Sep 25 '19 at 22:01

Dynamically-allocate a "copy" of std::cin by grabbing its buffer. Storing the memory in a std::unique_ptr would also be ideal as you wouldn't have to worry about manually deleting the pointer.

#include <memory>

int main(int argc, char* argv[]) {
  std::unique_ptr<std::istream> currentStream( argc == 3
    ? std::make_unique<std::ifstream>(argv[2])
    : std::make_unique<std::istream>(std::cin.rdbuf())
  );

  // will only fail when the file cannot open
  if (!currentStream) {
    std::cerr << "FILE COULD NOT BE OPENED\n";
    return 1;
  }

  std::string myLine;
  // go line by line and translate it
  while (std::getline(*currentStream, myLine)) {
    std::cout << rot13(myLine) << std::endl;
  }
}

score 1 · Answer 2 · answered Sep 25 '19 at 23:16

There are several places where your code could be improved. I think the most prominent place for improvement is that you are trying to do too much in one function. Your goal of not duplicating code is good, but modularize your approach. Move the shared code to its own function, as in:

void do_stuff(std::istream & currentStream)
{
    std::string myLine;
    // go line by line and translate it
    while (getline(currentStream, myLine)) {
        if (currentStream.eof()) {
            break;
        }
        std::cout << rot13(myLine) << std::endl;
    }
}

This function should contain everything that is shared between the two code paths. (I changed the pointer to a reference so that callers immediately know that a null pointer is not acceptable.) When you change your main function so that it calls this one, you should notice that a few things become easier. In particular, there is no need for dynamic allocation (which leads to no attempt to delete &cin – that looked bad). You can readily use a local variable for your file stream.

int main(int argc, const char ** argv)
{
    if (argc == 3) {
        // Handle optional file input
        std::ifstream fileStream(argv[2]);
        fileStream.open(argv[2]);
        if (fileStream.fail()) {
            std::cerr << "FILE COULD NOT BE OPENED\n";
            return 1;
        }
        do_stuff(fileStream);
        fileStream.close();
    } else {
        do_stuff(std::cin);
    }
    return 0;
}

By moving the common code to a separate function, you get to stay in your if clause. There is no need to deduce whether or not *currentStream needs closing, since you never left the code branch that created the file.

There is another place where you could simplify things. Don't call open and close. You are using the ifstream constructor that takes a file name, so the constructor already calls open for you. (When you explicitly call open, you are telling the computer to close the file and re-open it.) Similarly, the destructor will call close for you; that is a key point of RAII.

Getting rid of the unneeded calls leaves:

int main(int argc, const char ** argv)
{
    if (argc == 3) {
        // Handle optional file input
        std::ifstream fileStream(argv[2]);
        if (fileStream.fail()) {
            std::cerr << "FILE COULD NOT BE OPENED\n";
            return 1;
        }
        do_stuff(fileStream);
        // Keep in mind that, even though there is no C++ code here, there is something
        // important being done after the call to do_stuff. Specifically, the destructor
        // for fileStream is called, which closes the file for you.
    } else {
        do_stuff(std::cin);
    }
    return 0;
}

jaypb · Answer 3 · 2019-09-27T01:06:26.473

0

Tas has the right approach in the comments. You can't call the method directly on currentStream, you have to call it on the cast interface.

ifstream* stream = dynamic_cast<ifstream*>(currentStream);
if (stream) {
     stream->close();
}

I also think you should probably change your code to not rely on dynamic cast, either a new interface or separate method.

edited Sep 27 '19 at 01:06

answered Sep 25 '19 at 21:57

jaypb

1,544
10
23

1) But, one doesn't need to even call `close`, if one is `delete`ing the allocated stream - destructor of `std::ifstream` will call it. 2) Technically, the asker asked about `currentStream->open(argv[2]);` line as well (which is unaddressed with this answer), but it, too, is superfluous, because the constructor is used. 3) This answer doesn't mention the possible UB of `delete currentStream;`. – Algirdas Preidžius Sep 25 '19 at 21:59
I think you meant "stream" instead of "ifstream" in your `if` condition and in the line that calls `close`? – JaMiT Sep 25 '19 at 23:20

score 0 · Answer 4 · answered Sep 25 '19 at 23:23

Simply extract a method:

void process(std::istream is) {
    string myLine;
    // go line by line and translate it
    while (getline(is, myLine))
        cout << rot13(myLine) << endl;
}

int main(int argc, char** argv) {
    if (argc == 3) {
        ifstream ifs(argv[2]);
        if (!ifs) {
            cerr << "FILE COULD NOT BE OPENED\n";
            return 1;
        }
        process(ifs);
    } else {
        process(cin);
    }
}

How do I access a pointer's subclass's methods if those methods are unique to the subclass?

4 Answers4