3

In a library I have a function which searches for a key in a database and return a non-const reference to an object. I want to handle the case in which the key is not found, which is typically caused by a mistake when calling the function. This situation is so bad that the program cannot continue, so I print a message to help to spot the bug and call exit(1). The problem is with the return statement which will never be executed in this case, but have to be there anyway. If it was a pointer I could just return nullptr; but with a reference? Should I do something like this pseudo code?

 Type & get(const Key & k) {
     if (my_db.key_exists(k)) {
       return my_db.at(k);
     }
     std::cerr << k << " not found\n";
     exit(1); 
     return *(new Type(some_dummy_parameters));
 }

It looks so awful! Maybe I should just avoid such a function. Please, let me know your opinion!

DarioP
  • 5,377
  • 1
  • 33
  • 52
  • You don't need the return statement, since you'll never reach the end of the function. (Some compilers might warn, however.) – James Kanze Apr 03 '14 at 16:41
  • @JamesKanze Yes, it was all about avoiding compiler warnings.. but since the amount of suggestion received I'm now considering to use exceptions. Even if they are not caught causing a core dump, they still look like a better way to quit the function. – DarioP Apr 03 '14 at 17:09
  • @DarioP I'm not disagreeing with the other comments on that. I tend to avoid `exit` completely in C++ (because it doesn't call the destructors of any local variables), and propagate errors up to `main`, from where I can return. – James Kanze Apr 04 '14 at 08:11
  • You probably should be using exceptions as the answers have indicated. However, from a technical point of view, you can solve the problem at hand by reversing the sense of your test. Use `!my_db.key_exists(k)` in the if-statement. – kec Dec 18 '14 at 23:02

4 Answers4

9

This situation is so bad that the program cannot continue, so I print a message to help to spot the bug and call exit(1)

No. If this code is part of a library, the library should not be the one deciding if the application should exit or not.

What if a file is open and needs to be closed, or some other resource needs to be cleaned up, or if the user of your DB class wants to log the error and continue doing something else?

The answer is anything but what you're doing now. Throw an exception, return an error code, etc. but don't close the application down within library or class code.

Believe it or not, there was a commercial DB library that did exactly what you're doing (closing the app down). They got a lot of angry responses from users of their library as to why they're shutting the application down unexpectedly. And you know what -- the answer given to the customers was that "we felt the error was severe enough to stop the application, because our library can't continue to work properly". That is not only bad reasoning, it borders on arrogance, and the customers let them know that.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • I could throw an exception, but I cannot force the user to catch it. If for whatever reason he doesn't, this will become a core dump. Is this better than a call to exit()? – DarioP Apr 03 '14 at 16:40
  • 1
    If you clearly document what exceptions are thrown from your functions, and the user refuses to read the docs and doesn't do a try/catch, then that's their issue if their program crashes, not yours. – PaulMcKenzie Apr 03 '14 at 16:42
  • I like this answer, personal experience and anecdotes make you reason more that people telling "do this and that"! – DarioP Apr 03 '14 at 17:22
4

Exceptions

This is a common situation in many programs. To overcome this, Exceptions are used.

  • To handle unexpected situations, new Exceptions are created and "thrown" from the code.
  • Then they have to be "caught" by the program calling the function.

You can read more about exceptions here.

Hope this helps.

Tanmay Patil
  • 6,882
  • 2
  • 25
  • 45
3

The answer, as the other respondents, said, should be: throw an exception...

Type & get(const Key & k) {
     if( !my_db.key_exists(k) ) {
          std::stringstream error;
          error << "key " << k << " not found";
          throw std::runtime_error(error);
     }
     return my_db.at(k);
}
Massa
  • 8,647
  • 2
  • 25
  • 26
1

A library should never exit the hostest application.

use "return null", get into an "inconsistent state" whichin to every call you return NULL.
the library user will have to handle it.

or exceptions...

evenro
  • 2,626
  • 20
  • 35