returning null or throwing an exception?

Question

I have the following types:

typedef QPair < QTime , QTime > CalculatedTimeSlotRange;
typedef QList < CalculatedTimeSlotRange > CalculatedTimeSlotRangeList;
typedef QHash < quint8 , CalculatedTimeSlotRangeList > TimeSlotsTable;

I have a function like the following:

const CalculatedTimeSlotRangeList* TimeSlots::getCalculatedTimeSlotRangeList(const quint8 id) const
{
    QHashIterator<quint8,CalculatedTimeSlotRangeList> it(mTimeSlotsTable);
    while (it.hasNext()) {
        it.next();
        if(it.key() == id) {
            return &it.value();
        }
    }
    return NULL;
}

as you can see my function returns a NULL if it fails to find a key that matches id. Is this correct? or should I just throw an exception if the key does not exist? how should i throw an exception for this situation?

EDIT:

after seeing the comments and the answer I thought it's important to point out that it doesn't matter if an exception is thrown or the null is returned. This event implies incorrect parameters which are supplied to program through a number of files. The program must display an error message and ask the user to replace the parameter files with correct ones. So what is the better choice here? exception or null pointers since both of them mean the same thing in this context. Please edit the question and description if it fails to reflect my actual intention.

"Correct" really depends on the context. Is not finding the key an exceptional situation? A regular occurrence? Does the rest of the software use exceptions or not? — Timo Geusch, Aug 20 '14 at 04:18
One way to look at it is this: If the key *must* be there, but for some odd reason it isn't, then throw an exception. If it is perfectly reasonable for a key not to exist, then return NULL. — PaulMcKenzie, Aug 20 '14 at 04:19
@TimoGeusch yes! not finding a key means incorrect configuration which is done through a bunch of binary files that contain some parameters. — max, Aug 20 '14 at 04:20
@PaulMcKenzie no the key must exist. could you please tell me how to provide an exception. I'm kind of a noob in this case. — max, Aug 20 '14 at 04:21
@MoKi http://stackoverflow.com/questions/8480640/how-to-throw-a-c-exception — PaulMcKenzie, Aug 20 '14 at 04:23
Throw an exception in *exceptional* (i.e. generally unexpected) situations. — Jonathon Reinhart, Aug 20 '14 at 04:27
Btw, iterating over the QHash is an inefficient way to look up a value; if possible, call mTimeSlotsTable.find(id) instead; that way your lookups will be O(1) rather than O(N). — Jeremy Friesner, Aug 20 '14 at 04:38

score 1 · Accepted Answer · answered Aug 20 '14 at 04:31

Ask yourself: if you were someone using this function, which of the behaviors would you prefer that it exhibit? Each of the two options has its benefits and drawbacks -- If you throw an exception, then the onus is on the calling code to know about that possibility and to catch the exception at some point, otherwise the program will abort(), which to a user looks pretty much the same as a crash.

On the other hand, if you have your function return NULL, then the onus is on the caller to check the returned value for NULL-ness before continuing, or he risks crashing the program due to a NULL-pointer dereference. Also, dealing with functions that return a pointer-to-an-object can be confusing with respect to object-ownership issues -- the caller has to ask himself, "am I supposed to delete this object when I'm done with it, or not?" -- and getting the answer to that question wrong will introduce either a crash or a memory leak.

There is also a third option that you may not have considered: Return a reference to a dummy-object. Implementing the function using that option might look like this:

const CalculatedTimeSlotRangeList & TimeSlots::getCalculatedTimeSlotRangeList(const quint8 id) const
{
   QHashIterator<quint8,CalculatedTimeSlotRangeList> it(mTimeSlotsTable);
   while (it.hasNext()) {
       it.next();
       if(it.key() == id) {
           return it.value();
       }
   }

   static const CalculatedTimeSlotRangeList _dummyObject;  // note:  static to avoid dangling reference when returned!
   return _dummyObject;
}

I like this approach because now the user doesn't have to worry about checking for NULL, and he also doesn't have to worry about correctly handling an exception. This function will always return a reference to a valid CalculatedTimeSlotRangeList; in the not-found case, it will be an empty/default-constructed one; which (depending on your particular use-case) may be perfectly acceptable.

it doesn't matter if an exception is thrown or a null is returned. in this context it implies an invalid configuration parameter upon which the program will show an error message and will ask for correct parameter files. The program should not proceed any further. So which choice is more appropriate in terms of a good programming style? by the way nice explanation and thanks for the third alternative. — max, Aug 20 '14 at 04:42
Good programming style is often a matter of taste; there is no objective "right way to do it", only approaches that give you behavior closer to (or not-so-close to) the behavior you'd like your API to have. For example, I personally find it difficult to prove to myself the correctness of a program that throws exceptions, so I try to avoid them in my APIs when possible. Other people find manually checking return values to be tedious and error-prone, and so they prefer to throw exceptions. Either approach can be made to work well, though. — Jeremy Friesner, Aug 20 '14 at 04:54

score 0 · Answer 2 · answered Aug 20 '14 at 04:56

Since the failure condition indicates a fatal condition with the program configuration, out of the options you suggested I would definitely suggest throwing an exception, since it's a truly exceptional situation.

I think invalid_argument makes sense here, so you would #include <stdexcept> and then replace your return statement with throw std::invalid_argument("your error message about the bad configuration");

returning null or throwing an exception?

2 Answers2