Re: [boost] [local] Review (and resignation from review assistant)

Subject: Re: [boost] [local] Review (and resignation from review assistant)
From: Lorenzo Caminiti (lorcaminiti_at_[hidden])
Date: 2011-11-21 10:31:09

On Mon, Nov 21, 2011 at 9:56 AM, Joel de Guzman
<joel_at_[hidden]> wrote:
> On 11/21/2011 10:41 PM, Andrzej Krzemienski wrote:
>>>> - What is your evaluation of the design?
>>
>>> The library is very well designed.  The function declaration syntax looks
>> a little foreign at first, but it is very easy to get used to.
>>  Furthermore, error messages are usually able to point you to what you did
>> wrong.  The fact that you can write the body of the function in plain C++
>> code is a major advantage of this library.  Some have referred to
>> Boost.Bind, Boost.Lambda and Boost.Phoenix as essentially being equivalent
>> to this library, but in none of those libraries can you express the body of
>> your function in plain C++;  one advantage of this is that errors in your
>> code result in normal C++ error messages rather than cryptic error messages.
>>
>> Lorenzo, given this comment by Greg, I do have one more suggestion for
>> documentation (or did someone already say it?). Given the heated discussion
>> of whether Boost.Bind, Boost.Lambda and Boost.Phoenix are sufficient to
>> render Boost.Local unnecessary, perhaps it would be worth to provide a
>> comparison of how my code (not too trivial, and not too complex) would look
>> in either case. For instance:
>>
>>
>> With C++11 lambdas:
>>
>> for_each( vec.begin(), vec.end(), []( std::string & n ){ if(n.empty()) n =
>> "n/a";} );
>>
>>
>> With Boost.Lambda + Boost.Bind:
>>
>> for_each( vec.begin(), vec.end(), if_then(boost::bind(&std::string::empty,
>> _1), _1 = "n/a") );
>>
>>
>> With Phoenix:
>>
>> using boost::phoenix::arg_names::arg1;
>> using boost::phoenix::if_;
>> using boost::phoenix::bind; // right?
>>
>> for_each( vec.begin(), vec.end(), if_(bind(&std::string::empty, arg1)) [
>> arg1 = "n/a" ] );
>
> Better:
>
>  for_each( vec.begin(), vec.end(), if_(empty(_1)) [_1 = "n/a" ] );
>
>> With Boost.Local (Boost.Closure):
>>
>> BOOST_CLOSURE_PARAMS( std::string & n ) {
>>     if( n.empty() ) n = "n/a";
>> }BOOST_CLOSURE_NAME(replace_empties);
>>
>> for_each( vec.begin(), vec.end(), replace_empties );
>>
>>
>> Ok, I am not sure if there are no better ways to write it in each variant,
>> and I may be biased. I also didn't try to compile the examples. But it
>> should give the potential users an overview of options they have.
>
> Yep. This highlights the verbosity well:
>
>  for_each( vec.begin(), vec.end(), if_(empty(_1)) [_1 = "n/a"] );
>
> vs:
>
>  BOOST_CLOSURE_PARAMS( std::string & n ) {
>    if( n.empty() ) n = "n/a";
>  }BOOST_CLOSURE_NAME(replace_empties);
>
>  for_each( vec.begin(), vec.end(), replace_empties );

IMO, I'd be better if the example had more than 1-line of code in its
body. In the docs, I have an example to show the different approaches
that has 2-lines and I'd still think more than 2 statements would be
better:

https://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Alternatives.html#boost_local.Alternatives.local_functions.examples

C++11 lambdas:

    std::for_each(v.begin(), v.end(), [&sum, factor](double num) {
        sum += factor * num;
        std::cout << "Summed: " << sum << std::endl;
    });

Boost.Local:

    void BOOST_LOCAL_FUNCTION_PARAMS(double num,
            bind& sum, const bind& factor) {
        sum += factor * num;
        std::cout << "Summed: " << sum << std::endl;
    } BOOST_LOCAL_FUNCTION_NAME(add)
    std::for_each(v.begin(), v.end(), add);

Boost.Phoenix:

    std::for_each(v.begin(), v.end(), let(_f = cref(factor))[
        ref(sum) += _f * _1,
        std::cout << val("Summed: ") << ref(sum) << "\n"
    ]);

HTH,
--Lorenzo