the Switch library by Steven Watanabe has been accepted provisionally
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@...), 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@...) 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
participants (1)
-
Stjepan Rajko