Re: [boost] Boost Local review

Subject: Re: [boost] Boost Local review
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2011-11-20 06:19:44

On Sat, Nov 19, 2011 at 10:13 PM, Edward Diener <eldiener_at_[hidden]> wrote:
>
> This is my review of the Boost Local library by Lornezo Caminiti.

Hello Edward and thank you very much for your review.

>> Please state clearly whether you think this library should be accepted as
>> a
>> Boost library.
>
> First I vote for the Boost library to be accepted into Boost.

:)

>> Other questions you may want to consider:
>>
>> - What is your evaluation of the design?
>
> I think the design is very logical, easy to understand, and works very
> orthogonally. The implementer has put much thought into making Boost local
> easy to use, while retaining some advanced functionality.
>
> I see the local functions having the most importance by far, as an
> alternative to Boost lambda ( Boost Phoenix ) and C++0x lambda. Personally I
> do not see myself having much use for local functions but they do provide an
> alternative arsenal in software design and they do emulate the local
> function syntax in other languages fairly well. While I personally see
> little practical use for local functions in software design I would not want
> to vote against a good C++ implemementation of them, and I have no doubt
> after reading the documentation and trying out the code that the implementor
> of Boost local has done an excellent job of providing an analogy to local
> functions in C++.
>
> I see almost no use for local blocks since C++ already offers the ability to
> create blocks within a function by just surrounding soem code with a pair of
> braces ( { } ). I do understand that local blocks offer some binding
> abilities for variables outside the scope of the block but I do no believe
> this binding ability offers much functionality although I do understand that
> if offers orthogonality with local functions.

OK. As I mentioned before, I'm fine with removing local blocks from
Boost.Local because they can be _trivially_ implemented using local
functions by a user that needs them:

int x = 10, y = x;
void BOOST_LOCAL_FUNCTION(const capture x, const capture y) {
    assert( x == y );
} BOOST_LOCAL_FUNCTION_END(block1)
block1();

Instead of:

int x = 10, y = x;
BOOST_LOCAL_BLOCK(const capture x, const capture y) {
    assert( x == y );
} BOOST_LOCAL_BLOCK_END

> It would be nice if local exits and scope exits could come together as a
> single methodology, but I see no reason for both of them not existing in
> Boost if they can not. Once again I see little personal use for scope
> exits/local exits since my viewpoint of design generally precludes such
> constructs, which I view somewhat as hacks to make up for poor design. But
> again I admire the way the implementor of Boost local has designed local
> exits to be orthogonal with local functions and local blocks.

I understand. At this point it looks like that:
1) Local blocks are removed (because trivially implementable using
local functions-- see above).
2) Local exits are remove (because void and this_ are added to the
existing ScopeExit-- see other thread).
Then Boost.Local is renamed Boost.LocalFunction and it remains about
its meat which definitely is local functions. I'd be happy with that.

> The use of macros may be seen as somewhat unfortunate but the ease of using
> the macros makes the library easy to use. I do not think that having macros
> use two possible interfaces in any way reflects badly on the library, but
> instead applaud the implementor of the library in this respect. The
> implementor has clearly done an excellent job of making the use of his
> macros easy regarding their syntax.
>
>> - What is your evaluation of the implementation?
>
> I only briefly looked at it but from what I saw it is meticulous and very
> well done.
>
>> - What is your evaluation of the documentation?
>
> The documentation is clear and really excellent.
>
>> - What is your evaluation of the potential usefulness of the library?
>
> For me personally, because of my programming style, I see little use for the
> library. But for others, who want local function syntax and are used to
> cleanup code in functions Boost local obviously offers a very easy syntax to
> use. I would still encourage others to look at lambda functions, both in
> Boost and C++0x, before they look at local functions for solving design
> issues, but if they do choose local functions as part of the way they want
> to design some of their code, Boost local offers an excellent solution.

I agree. The tool choice should be left to our library users (which
can consider their personal preferences as well as they have clear
visibility on the requirements of the problem their are trying to
solve). That is also why I present and compare all the alternatives
that I know in the Alternatives section:
https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Alternatives.html

>> - Did you try to use the library?  With what compiler?  Did you have any
>> problems?
>
> I tried with the latest version of gcc and VC++ on Windows and everything
> just worked.
>
>> - How much effort did you put into your evaluation?  A glance?  A quick
>> reading?  In-depth study?
>
> I put a moderate amount of effort reading the docs and trying out the
> library. I also looked at some of the implementation, particularly the use
> of macros.
>
>> - Are you knowledgeable about the problem domain?
>
> I am knowledgable about the uses of local functions and also the uses of
> Boost PP, my own VMD library, and C++ macros in general.
>
>>
>> Please also consider the following issues and proposals specific to
>> Boost.Local.  Your opinion is welcome on any or all of these.
>>
>> - Boost.Local's local exits provide the same functionality (and then some)
>> as Boost.ScopeExit.  Does this duplication of functionality need to be
>> dealt with, and if so, how?
>
> If local exits and scope exits could be combined, that would be fine, but I
> do not think this is absolutely necessary to accept the library. I never
> find that having more than one way of doing anything is detrimental as long
> as each similar implementation is different in some basic respects.
>
>> - Lorenzo is proposing to add boost::local::function::
>> overload to Boost.Function as boost::function::overload.  See
>>
>> https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost/local/function/overload.html
>
> I do not think this should be part of boost::function but possibly part of
> boost::utility. Simply because one is providing an interface for overloading
> a set of functors using templates does not mean it belongs as part of
> boost::function.
>
>> - Lorenzo is proposing to add BOOST_IDENTITY_TYPE to boost/utility.  See
>>
>> https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_IDENTITY_TYPE.html
>
> Yes, I think this should be done. If it is not done the name of the macro
> should be BOOST_LOCAL_IDENTITY_TYPE.
>
>> - Likewise, Lorenzo is proposing to add BOOST_IDENTITY_VALUE to
>> boost/utility.  See
>>
>> https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/BOOST_IDENTITY_VALUE.html
>
> Yes, I think this should be done. If it is not done the name of the macro
> should be BOOST_LOCAL_IDENTITY_VALUE.
>
> In general all macros in Boost local needs to start with some basic
> identifying prefix and BOOST_LOCAL_ is the natural prefix for that.
>
>> Lastly, please note that Boost.Local has included a copy of the Variadic
>> Macro Data (VMD) library in boost/detail/preprocessor/variadic_macro_data.
>> Since then, VMD has been modified and integrated into Boost.Preprocessor.
>> After the review has concluded, Lorenzo will remove this local copy of the
>> VMD library and replace its usage within Boost.Local with the
>> Boost.Preprocessor variadic extensions.
>
> As the author of VMD I completely agree with this.

Thanks for VMD (and Boost.Preprocessor variadics extensions)--  it was
very useful to support the variadics syntax.

Thank you very much for your comments.
--Lorenzo