[boost] the Switch library by Steven Watanabe has been accepted provisionally

From: Stjepan Rajko (stipe_at_[hidden])
Date: 2008-01-17 14:57:39

The Switch library, submitted by Steven Watanabe, has been accepted
provisionally pending a mini-review to be scheduled when possible.
The purpose of the mini-review will be to assure that the suggestions
outlined below have been adequately addressed.

I would like to again thank Steven for submitting a very useful
library, and for diligently working with the reviewers on improving
its design.  I would also like to thank Tobias Schwinger, Joel de
Guzman, Darren Cook, Daniel (dherring_at_...), Felipe Magno de Almeida,
and Paul Bristow for providing reviews, votes and half votes.
Finally, thanks also to Dan Marsden, Larry Evans, Alexander Nasonov,
Richard (legalize_at_...) and Mathias Gaunard for contributing to the
discussion.

SUMMARY OF VOTES:

Tobias Schwinger and Joel de Guzman have submitted full reviews with
positive votes:

http://article.gmane.org/gmane.comp.lib.boost.user/32637
http://article.gmane.org/gmane.comp.lib.boost.user/32634 (Joel
eventually converted his vote to "yes")

One "yes" (half) vote was submitted by Paul Bristow:
http://article.gmane.org/gmane.comp.lib.boost.devel/170069

One "no" (half) vote was submitted by Felipe Magno de Almeida:
http://article.gmane.org/gmane.comp.lib.boost.devel/170051

Another "no" came privately - the point there was that the library
didn't undergo enough scrutiny before submission, and that it should
be resubmitted.

Another privately submitted "no" came soon after the review opened,
stating that the reviewer was not able to deduce any
usability/usefulness from the library documentation, and suggested
there should be a motivating example on the first page.

RATIONALE FOR ACCEPTANCE:

I believe the low number of full, positive reviews is justified by the
relatively narrow scope that this library covers.  Within that scope,
however, reviewers indicated that the library is extremely useful.
Also, the reviewers submitting positive votes have indicated (and
proven) significant expertise in the area, leading me to place much
weight on their votes.  On the other hand, the negative votes did not
point out any concrete flaws in the library itself that would merit
rejection.

RATIONALE FOR REQUIRING A MINI-REVIEW

The "no" votes did, on the other hand, indicate some skepticism of the
quality of the library/documentation as submitted, and the progress of
the redesign that was happening during the review.  Also, as a result
of the review, the potential scope of the library has expanded
significantly from what was originally proposed (which mostly focused
on the ability to choose the number of cases at run-time).  I believe
that a combination of these two give sufficient reason for a
mini-review - doubts about the library can be resolved, and those that
might become prospective users of the library under the expanded scope
will get a chance to give their opinion.

I volunteered to manage the mini-review, so once the library is ready
it will hopefully not remain in the review queue for too long (as it
seems that finding review managers is the biggest problem ATM).  To
keep the burden on the community low, I propose to treat the
mini-review as a partial continuation of the current review - I will
consider the "yes" votes by Tobias and Joel to be left standing unless
they state otherwise, and will require any "no" votes to be
(re)justified given the new condition of the library.

SUGGESTED CHANGES TO THE LIBRARY:

* Documentation

All reviewers have noted that the documentation could be improved.
Specific suggestions were:
 * motivating example on the first page
 * additional examples
 * performance stats
 * more explanation of the types involved and how they are used.

As the scope for the library has increased as a consequence of the
review, a gentler, more detailed documentation approach might benefit
the library - I believe that many people might find the new library
useful, given the right motivation and examples in the documentation.

* Return type

Tobias Schwinger suggested that the return type should be specified
explicitly in the switch_ call, rather than inferring it from the
function object, and this seems to be the status quo in the newer designs.
Joel de Guzman also suggested that if a version of switch_
should infer the return type, that return type should be a
Boost.Variant of the types returned by each of the cases.

* Default default behavior

If the default behavior is left unspecified, switch_ should invoke the
default constructor of the return type in the default case, rather
than throwing an exception.  This was suggested by Tobias Schwinger
after the issue was raised by Dan Marsden.  Alexander Nasonov has also
indicated that a throwing default case can cause inefficiency, even
when it is guaranteed not to be invoked.

* Fall-through

Alexander Nasonov pointed out the lack of fall-through capability in
the submitted implementation.  Fall-through has been incorporated into
the recent designs being discussed.

* Allowing void-returning function objects for the default case with a
non-void returning switch

Tobias Schwinger mentioned the possibility of allowing default case
function objects of void return type even when the switch return type
is non-void, as long as the function object call does not return. This
would be useful, for example, if the function object is guaranteed to
throw.  After some discussion on whether determining if the return
type is void can be done efficiently, it looks like the proposed
solution is to use a special return type that indicates that the call
will not return.

Since Tobias indicated that this feature is a minor point, it could be
implemented or not, depending on how well it fits into the final
design.

* Design

There has been a lengthy discussion on the underlying design of the
library, what it should offer, and whether the name of the library is
appropriate.  There are two types of designs that have been put
forward as being appropriate and/or necessary for a Boost.Switch
library.  On one hand, there is the submitted design (labeled "A" in
recent postings), which takes the switch cases in an MPL sequence and
a single function object containing implementations of all cases. Joel
de Guzman has proposed another design (labeled "B"), in which the
switch cases are specified separately and allow a different function
to be used for each case.  Design "A" is better suited for situations
in which the number of cases is determined by a compile time constant,
while "B" is more appropriate for many use cases, and provides more
flexibility and a more familiar syntax.  The result of the discussion
was that the Boost.Switch library should cover both situations, and an
interface is now being developed that can be used in different ways
for different uses (based on a common concept capturing case
sequences).

Thanks again to all that have contributed to this review, and
congratulations to Steven for the accepted library.  As others have
stated, there is much confidence that Steven will meet the challenges
successfully for the mini-review, which we will schedule as soon as
the library is ready.

Regards,

Stjepan