[boost] [review] Review of Boost.Exception

From: Dean Michael Berris (mikhailberis_at_[hidden])
Date: 2007-09-30 04:02:57

Hi Everyone,

Below is my review of the proposed Boost.Exception library.

    * What is your evaluation of the design?

I personally like that the exception library is implemented in just a
couple of header files, without having to drag in a lot of other
libraries. It is succinctly implemented, though I have some
reservations about some design decisions:

get_error_info(T const &) returns a pointer instead of a const copy of
the actual error_info it gets. This can be a potential headache to
deal with especially in examining the error_info associated with the
thrown exception. A proposed change would be to have get_error_info(T
const &) return a const copy of the data, while get_error_info_ptr(T
const &) would return a pointer as in the current version.

Although it might also take quite some effort to remove the runtime
polymorphic behavior which relies on shared_ptr<>'s and virtual tables
with info_base, perhaps an expression templates-based approach to
building exceptions that derive from a common base could mitigate
these (theoretical) inefficiencies introduced with runtime polymorphic
behavior? A linear inheritance encapsulating the different information
using something like Boost.Fusion's or Boost.MPL's `fold` concept to
create an exception type using expression templates might work. Then I
haven't tried this myself, so it might be worth considering in the
future.

My concern with the use of shared_ptr<>'s is that in cases where an
exception pertaining to "out of memory" conditions would arise,
throwing a boost::exception and then having to allocate more memory
just to encapsulate additional information would lead to undesirable
termination. So keeping exception data statically allocated on the
stack instead of being in the heap might be a good alternative. This
is just my thinking though I don't have enough data to back this up.

    * What is your evaluation of the implementation?

It's very cleanly implemented, and very much readable.

    * What is your evaluation of the documentation?

The documentation is very well written although an alternative example
aside from the case with file IO would be greatly appreciated. That
said, the example given is sufficient enough to make a case for the
utility of the library.

    * What is your evaluation of the potential usefulness
      of the library?

In cases where minimal memory footprints are required, the current
implementation introduces a lot of unnecessary allocations on the heap
when additional information is being encapsulated into the exception.
An expression templates-based approach as described above (though
untested) should be able to mitigate this consequence.

In all other cases where efficiency and performance are not critical,
I can see this exception library to be very useful.

    * Did you try to use the library?  With what compiler?
      Did you have any problems?

Not yet, though I see myself using this in a project really soon.

    * How much effort did you put into your evaluation?
      A glance? A quick reading? In-depth study?

I went into the implementation details, and the conceptual design
based on the source code. I did a quick read of the documentation,
which didn't seem to change since the first time this library has been
introduced.

    * Are you knowledgeable about the problem domain?

I have been writing ad-hoc exception hierarchies based on
std::exception for quite a while already, and I can say that this
implementation of exceptions is a welcome development.

    * Do you think the library should be accepted as a Boost library?
      Be sure to say this explicitly so that your other comments
      don't obscure your overall opinion.

I think yes, it should be accepted as a Boost library. I can see it
getting a lot of improvements in terms of implementation though, but
being in the Boost C++ Library will allow more people to be able to
propose changes and eventually extend this library in many different
ways.

-- 
Dean Michael C. Berris
Software Engineer, Friendster, Inc.
[http://cplusplus-soup.blogspot.com/]
[mikhailberis_at_[hidden]]
[[phone]]
[[phone]]