Why is throwing a non exception considered bad design?

Question

I have some code to sort a vector of objects. If any of the objects is invalid I want to stop sorting immediately and report an error. In the error, I want to include the description of an invalid object (no matter which one if there are many).

This is my code (not complete, but I hope you can follow me):

int sortProc(const Bulk & b1, const Bulk & b2) {
    if (!b1.isValid()) throw b1;
    if (!b2.isValid()) throw b2;
    return b1.compareTo(b2);
}

vector<Bulk> * items = getItems();
try {
    sort(items->begin(), items->end(), sortProc);
} catch (const Bulk & item) {
    cout << "Cannot sort item: " << item.description();
}

Now, I'm a bit unsure of my code because I've heard that all exceptions should subclass the exception class and it's considered bad practice to throw objects that are not instances of exception, but I don't really understand why. My code above works, is anything wrong with it? This is a serious question, so if you see any problems I'd be glad to know. I'm not looking for moral concerns.

Answer 1

Here is code very similar to yours, except it doesn't have random-access flow control:

std::unordered_set<std::string> errors;
auto sortProc = [&errors](const Bulk & b1, const Bulk & b2)->int {
  if (!b1.isValid()) errors.insert(b1.description());
  if (!b2.isValid()) errors.insert(b2.description());
  if (!b1.isValid()||!b2.isValid())
    return b1.isValid()<b2.isValid();
  return b1.compareTo(b2);
}

vector<Bulk> * items = getItems();
sort(items->begin(), items->end(), sortProc);
if (!errors.empty()) {
  std::cout << "Unsortable items found while sorting:\n";
  for (auto const& e : errors) {
    std::cout << e << "\n";
  }
}

I prefer avoiding random access flow control except in exceptional circumstances. The sort function could continue oblivious to the failure of an element to be isValid, so it should by default rather than have interface decoupled exception based flow control.

While it is tempting to use exceptions as a way to return a "second return value", the result is that your code's interface becomes obscured and coupled with your implementation details of both your code, and the code that uses your code.

Depending on the program, truly exceptional circumstances might vary from "failure to allocate memory" through to "hard disk failure", where there is no way to recover or have your code act error-oblivious.

Note that you can even make the above code less coupled by noting that invalid elements are sorted first or last. Then after the sort, examine your list, and check if there are invalid elements that have been collected there, rather than relying on a log of cumulative errors.

Answer 2

I see a (maybe serious) problem having Bulk transporting input and failure data. Having a BulkException derived form std::exception is way cleaner!

While having a state indicating a failure is fine, I think using the class to transport the failure is no good.

A BulkException could gather additional information (like a stack trace) which is useless for normal operation.

Answer 3

Good design is about quality. Is your code provably correct? Does it minimize dependencies on external objects? is it reusable? Is it understandable? Does it eliminate redundancies? Can you test it? Can other people read it and know what it does? That last question is really hard, because you are not other people - you're the author.

Answer 4

Why is throwing a non exception considered bad design?

Because rarely do systems exist in a vacuum.

Exceptions serve a fundamental purpose: to generate a packet of information about an error or some other "exceptional" condition, and propagate that information across boundaries without regard to the locality of those boundaries.

Consider the last part carefully:

without regard to the locality of those boundaries.

You can think of an exceptional condition as one which must be attended to in some way. If there is a piece of code that can handle it, then that code should be given the opportunity. If no code exists which can attend to it, then the program must die.

In this context, the packet of information describing the exceptional condition must be free to flow through any part of the program -- even those that you did not personally write, or even thought might one day be written when your project was a mere glimmer in your eye. In order for this to work however, all exceptions must be written using a familiar protocol. That is to say, far-flung exception handlers must be able to understand at least the basic information contained in the packet. Everybody has to be speaking the same language.

The way this is generally accomplished in C++ is by deriving all exception objects from std::exception. This way, a handler in a far-flung part of the code -- perhaps even code you never even dreampt of writing -- can at the very least report on the exceptional occurrence before the program meets with it's demise. It might even be able to handle the situation and allow the program to live on, but this is often not possible.

Answer 5

It's not about moral. It's not about functionality. It's not about correctness either. It's about the logic behind your code.

Remember code reflects how you think. A clear minded developer would never throw an "exception" which is indeed not an exception cause that just confuses what the logic is.

Joker_vD is also right regarding the productivity of the code but I don't think you are there yet.

Answer 6

I'm not looking for moral concerns.

You can't ask a style question then ban all answers based on "moral concerns", if you expect to figure it out.

Some people think that throwing only objects of types deriving std::exception provides consistency of interface, since you can invoke .what() on all of them and catch them all together at the top level of your program. You can also guarantee that other translation units — those who have never heard of your class Bulk — will be able to catch the exception if they want to (if only as a std::exception).

• Is your program wrong? No.

• Does it work? Yes, of course it does.

But sometimes simply "working" is not considered enough and we like to be a little more tidy about things.

That's really it...