1

I have a function in C++ that reads from a file. The functions takes as parameter a string which represents the name of the file. The function has an if condition to check for errors (such as file does not exist).

    void B::readFile(string file)
        {
            ifstream stfile;
            stfile.open(file.c_str(), ios::in);

            if ( stfile.fail() ) {
                cerr << "Unable to open input file" << endl;
                exit(-1);
            }

           //---goes on here

         }

My question is:

  1. Is the above implementation a correct way to check for errors, or should I throw an exception? Do you have a reason for choosing either one?
  2. If the above implementation is correct, for testing: Should I make the method return a string ("Unable to open input file"), and ASSERT that this string is returned or not in the test?
1
  • This isn't really anything to do with reading from a file.
    – Peter Wood
    Mar 18, 2013 at 19:46

3 Answers 3

2

1)What do you want the app to do? Is the failure recoverable at a higher level? If it is, throw an exception. If it's not, and this is code fairly early on, just exiting with an error can be ok, but you should probably add the filename that failed to the log message.

2)Definitely not. You could make it return a value such as true/false for success/failure, but you'd never want it to be a string like that. You'd want to return an error code when its expected that the file may not exist and likely that either the higher level code will handle it or not care. (Exceptions are expensive so for common conditions you shouldn't throw them).

1
  • Regarding my first question, is it true that C++ iostreams do not throw exceptions by default, and therefore try/catch would not be useful in this case?
    – FranXh
    Mar 18, 2013 at 20:04
2

Is the above implementation a correct way to check for errors, or should I throw an exception? Do you have a reason for choosing either one?

That implementation is correct if the error can be completely handled when it is detected. (You seem to be deciding that the program should exit)

You should throw an exception if the problem can not be completely handled at the location it's detected.

If the above implementation is correct, for testing: Should I make the method return a string ("Unable to open input file"), and ASSERT that this string is returned or not in the test?

ASSERT has no effect in production code. You shouldn't make your program's behavior depend on an ASSERT.

Focus on where the problem can be solved. If you want the calling code to decide how to handle this problem, throw.

Returning an error code is typical in C, which doesn't have catch/throw. But this practice punishes programmers for making small, concise functions because the error may have to be returned through many stack levels.

1

It all depends on what "clients" are going to use this method. Who is going to call this code, and what will they use it for.

If it's better for them to not have an exception thrown, then use your return format. As far as the message vs the number, it again depends on how people will use it.

I usually prefer throwing an exception over other ways of returning, because I can make a custom exception and add details to the exception so that the caller knows exactly what happened (so they can figure out how to fix it in their code).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.