Re: exception handling anti-patterns in documentation

From: Andrzej Krzemienski (akrzemi1_at_[hidden])
Date: 2022-05-30 08:11:30

pon., 30 maj 2022 o 01:59 Emil Dotchevski via Boost <boost_at_[hidden]>
napisał(a):

> On Sun, May 29, 2022 at 3:49 PM Gavin Lambert via Boost <
> boost_at_[hidden]> wrote:
> >
> > On 30/05/2022 02:10, Robert Ramey wrote:
> > > On 5/28/22 3:39 PM, Andrzej Krzemienski wrote:
> > >> try {
> > >>>    lib::function();
> > >>> }
> > >>> catch(lib::exception const&) {
> > >>>    // handle failure
> > >>> }
> > >
> > > Could you explain what's wrong with the above code?
>

The code itself need not necessarily be wrong. My concern is that it
appears in the context of the documentation of a Boost library. It is my
experience from 15 years ago -- maybe the situation is different now --
that there is no place or course where you can learn good C++: why value
semantics are important, why exceptions and exception safety is important,
why resource handling is important. You can program in C++ for 5 years, be
considered an "expert" in your workplace, and not know the foundations of
C++. In that case, you learn from what you got: libraries you use, and
reading the docs. This is how I learned good C++: by reading Boost docs,
and Bost discussion groups.

So, a programmer with 5 years experience in C++, who thinks they know C++,
and believe that "if library throws an exception you must immediately catch
it, or else the program will crash" (I was literally taught that in my C++
classes), sees an example like this, then the bat habit is immediately
reinforced: they will think, "ok, so Boost does this this way too, so it
must be the right way".

Also, I simplified it too much. The examples that I was referring to have
this pattern:

try {
   lib::type p = lib::function();
}
catch(lib::exception const&) {
    // handle failure
}

Now, it is almost impossible that someone -- experienced or not -- would be
writing a code like this. If you create an object `p`, it is because you
want to use it: read its value, or alter its state. And you cannot use the
object that immediately goes out of scope. So the real code would be

try {
   lib::type p = lib::function();
   use(p);
}
catch(lib::exception const&) {
    // handle failure
}

Or:

lib::type p = {};
try {
   lib::type p = lib::function();
}
catch(lib::exception const&) {
    // handle failure
}
use(p);

Which is usually exception unsafe, goes against the design of exception
handling, and is often the source of bugs. I have seen this pattern too
often in the programs that I am in charge of: literally with the comment
like this "// handle failure", and without no handling actually, only a
"TODO" comment.

> >
> > I believe the point was that if exceptions are always caught immediately
> > then it smacks of the "exceptions as control flow" anti-pattern, and a
> > non-throwing form of the library function ought to be available/used
> > instead.  Essentially, the exception is being used as an alternative
> > return value rather than as an actual exception.
> >
> > It's from the school of thought that exceptions should be "truly
> > exceptional" -- i.e. any condition that might theoretically raise an
> > exception can be explicitly checked in advance by a non-throwing method
>
> I'll let him speak for himself, but I'd be surprised if Andrzej's point
> comes from that school of thought, he is not a Rust programmer. :)
>
> The issue with such examples is that they are useless at best: an
> experienced programmer knows what to do, while a novice might think that he
> is always supposed to immediately catch exceptions, as may be the case in
> other languages (e.g. Java) which lack deterministic termination.
>
> The correct use of exception handling in C++ is to let exceptions propagate
> unmolested by default; there is no explicit checking for errors after
> calling a function that may throw. You catch only where the error can be
> handled and the program can restore normal operations. And while it is true
> that unhandled exceptions terminate the program, that is always a logic
> error, programs should handle all exceptions that may be thrown.
> Terminating the program when an exception is thrown is valid only under
> -fno-exceptions (in Boost you'd do that by providing a suitable definition
> for boost::throw_exception).
>

To a great extent I agree with Emil's view. although I would probably use
different words: you only catch (and not rethrow) an exception potentially
thrown by instruction X when you are sure the subsequent instructions do
not rely on the successful execution of X.

Reards,
&rzej;

> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>