* Are you knowledgeable about the problem domain?
I frequently use the RAII pattern in day-to-day programming, and I often advocate the use of this pattern as one of the major highlights of the C++ language.
* What is your evaluation of the potential usefulness of the library?
Boost has needed a library like this for a long time. I believe a library that does something similar to this should be one of the core language support libraries that all C++ programmers should use.
* What is your evaluation of the design?
My main concern here is that this library only provides a macro-ized version, and nothing analogous to Alexandrescu's ScopeGuard, which takes a functor. I think this is a serious problem. The documentation states, "But there are times when writing a special class for such variable is not worth the effort. This is when ScopeExit macro comes into play." For very small amounts of exit code, which is usually the case in my experience, a BLL lambda expression is perfectly suitable. The most common action to be taken, in my experience, is just a function call, which in fact only needs bind. The main reasons for my preference of ScopeGuard over the proposed interface for small exit code sequences are: -Lower ugliness factor. -Better cohesiveness with possibly more complicated code patterns. -Easier analysis by automated tools. RECOMMENDATION 1: Something comparable to ScopeGuard or the detail scope_guard should be included in this library. A secondary concern is that about 2/3 of the time I'm using RAII, the pattern is a "conditional free" of the type explicated in the documentation example:
if(!commit) m_persons.pop_back();
The pattern here is: 1) Initialize a variable. 2) Do some work. 3) Set a variable. 4) Free a resource if variable is not set. This process is tedious and error-prone, and very much distracting from the main work to be done. Minimizing the amount of distracting error code necessary is one of the most important jobs of the code writer. I understand that it is impossible to determine whether normal exit or unwinding has triggered a destructor, but I believe there may be other ways to encode this pattern, at least freeing the implementor from having to type an explicit 'if.' RECOMMENDATION 2: Support for the "conditional free" pattern should be added.
BOOST_SCOPE_EXIT( (commit)(m_persons) )
This construct is confusing to me. I believe this is a PP sequence, but I'm not sure. Since this library should be used in all parts of code, and not only by library implementors, I do not think it is appropriate for programmers to be exposed to the PP metaprogramming syntax. Normal function call notation should be used. RECOMMENDATION 3: Use familiar syntax.
* What is your evaluation of the documentation?
It needs to be expanded. The exact semantics of this library are very unclear to me. Each identifier should be formally specified, and the overall semantics explained. For example: -The parameter of BOOST_SCOPE_EXIT is not explained. -What constructs are permitted as parameters to the above macro? How many times are they evaluated? -Exception-safety guarantees, if any, are not explained. What happens if an exception is thrown from within the exit code? Also, there needs to be analysis regarding performance and storage efficiency impact, if any, as well as any other similar considerations. RECOMMENDATION 4: Expand documentation.
* 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.
This library would be a valuable addition, but it is incomplete in its present form. As it stands now, I probably would not use it, for the reasons stated above. However, the "long exit code" case (where both a specialized class and lambda expressions are inappropriate) does come up from time to time, so I might consider using it in those cases. I do not believe this library should be accepted until it has been expanded and re-reviewed with these use features and more complete documentation.