[boost] Boost Local review

Subject: [boost] Boost Local review
From: Edward Diener (eldiener_at_[hidden])
Date: 2011-11-19 22:13:00

This is my review of the Boost Local library by Lornezo Caminiti.

>
> 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.

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.

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.

> - 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.

Edward Diener