2018-04-09

Exceptions to control the flow

Tracking-data flow chart

Using exceptions to control the flow of your program is an anti-pattern. Don't take too seriously whoever believes in exceptions as an indispensable feature of a language. Better no exception than misused exceptions!

First we have our custom exception:

namespace Project
{
namespace Exception
{

class NotFound : public std::runtime_error
{
public:
    NotFound() : std::runtime_error("not found")
    {}
};

} // ns Exception
} // ns Project

Then we have a class, let me call it PtrCollector, which does things, and we don't really care about what it does, except (ops!) for a method which looks like this:

template <typename T>
class PtrCollector
{
// ...
public:
    T* find(T x) const {
        for (auto p = m_collection.begin();
             p != m_collection.end();
             ++p) {
            if (**p == x)
                return *p;
        }
        throw Project::Exception::NotFound();
    }
// ...
};

Now, you can say that we use it in the code in a way that it's a problem if we can't find our x in the collection. This implies that it's an exceptional situation that nonetheless can happen somewhere “deep inside”. You think so… until you find code like this:

    try {
        auto s = c.find(a_value);
        do_things_to_ptr(s);
    } catch (const Project::Exception::NotFound&) {
    }

    do_other_things_without_s();

This means that in that point of the code it's OK if we don't find a_value!

We are using exceptions to drive application logic, instead of doing this:

    auto s = c.find(a_value);
    if (s)
        do_things_to_ptr(s);
    
    do_other_things_without_s();

Of course find() must be rewritten like this:

    T* find(T x) const {
        for (auto p = m_collection.begin();
             p != m_collection.end();
             ++p) {
            if (**p == x)
                return *p;
        }
        return nullptr;
    }

Right: no exception!

We need just to check if s isn't a null pointer. It isn't a strange idea; think about the iterators, e.g. it != x.end().

Maybe in some other part of our code, in a deep dark place, we really don't expect a null pointer from find(), because there the value we search for should exist. Then we can do something like this:

    auto s = c.find(a_value);
    if (s == nullptr)
        throw Project::Exception::NotFound();

And we try-catch in the outermost sensible/reasonable place.

If they exist dark corners where using exceptions to control the flow is acceptable, this one isn't among those.

No comments:

Post a Comment