On 12/3/23 16:50, Andrzej Krzemienski via Boost wrote:
Hi Everyone, I want to share some thoughts regarding the review of Boost.Scope. The fact that it wants to implement 1-to-1 a feature from a TS -- not from the standard -- impedes the review process. The reviewer says, "this feature is bad". The author responds, "but this is how the TS requires it to be". And now what? Should we accept a bad feature because the TS requires so?
If you think a certain feature is badly designed or in any way harmful, you are free to reflect that in your review. That's what the review process is for. When I say that this or that is done so because of the TS, I'm explaining my reasoning behind doing it this way. This is not to say "we must accept the library because TS". If the community doesn't think compatibility with the TS is a worthy goal, then let that be the result of the review.
The move assignment operator in scope_exit and friends. It looks like its existence is against the goal of the feature. You define a guard and then you use it. Why would you assign a different guard to it?
Scope guards are *not* move assignable.
Why do we have scope_success? An alternative to using scope success is to just put your instructions at the end of the function. Do you have multiple return statements in your function and at the same time you need the same sequence of operations to be performed at each end? This is a good motivation to restructure your function to have a single return. Multiple returns are generally OK, but needing the same sequence warrants a single return.
What if having a single return is difficult to achieve? Sprinkling `goto finish` doesn't work well sometimes, e.g. when you have variable definitions in the middle of the function. It is also more prone to errors.
While I understand the existence of the triplet `scope_exit`, `scope_fail` and `scope_success` (D uses an analogous triplet), it takes a while to understand the difference between scope_exit and scope_final. And the choice of name doesn't help there. The Standard library chose to use term guard (std::lock_guard) to indicate something small and cheap. Maybe use this name instead. Not perfect either, but at least consistent with what we have in STD.
I could rename scope_final to something else, especially given that Peter has requested to rename BOOST_SCOPE_FINAL for BOOST_SCOPE_DEFER, but I haven't seen a good name for it. scope_guard sounds too generic to me, and it surely doesn't make the distinction from scope_exit any more obvious. scope_defer doesn't make much sense from the English point of view (though, I'm not a native speaker).