Re: [boost] [local] Review (and resignation from review assistant)
- 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" ] ); 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. Regards, &rzej
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 ); Regards, -- Joel de Guzman http://www.boostpro.com http://boost-spirit.com
On Mon, Nov 21, 2011 at 9:56 AM, Joel de Guzman <joel@boost-consulting.com> 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_loca... 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
On 11/21/2011 09:31 AM, Lorenzo Caminiti wrote:
On Mon, Nov 21, 2011 at 9:56 AM, Joel de Guzman <joel@boost-consulting.com> 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_loca...
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" ]);
Why not just: std::for_each(v.begin(), v.end(), ref(sum) += cref(factor) * _1, std::cout<< val("Summed: ")<< ref(sum)<< "\n" );
HTH, --Lorenzo
_______________________________________________ Unsubscribe& other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Mon, Nov 21, 2011 at 10:33 AM, Thomas Heller <thom.heller@googlemail.com> wrote:
On 11/21/2011 09:31 AM, Lorenzo Caminiti wrote:
On Mon, Nov 21, 2011 at 9:56 AM, Joel de Guzman <joel@boost-consulting.com> 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_loca...
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" ]);
Why not just: std::for_each(v.begin(), v.end(), ref(sum) += cref(factor) * _1, std::cout<< val("Summed: ")<< ref(sum)<< "\n" );
(An open parenthesis "(" is missing before "ref(sum) ...", correct?) Because I took your suggestion below too literally. I will make this simplification in the docs (BTW, as I asked in the thread below, if you or anyone else see any other improvement to the Phoenix, Lambda, etc code that I present in Local's docs, please feel free to point it out). -- from: http://lists.boost.org/Archives/boost/2011/03/179294.php -- On Sun, Mar 27, 2011 at 11:35 AM, Thomas Heller <thom.heller@googlemail.com> wrote:
On Sunday, March 27, 2011 05:43:39 PM Lorenzo Caminiti wrote:
Yes, but the use case would be to have factor const *only* within the "function" passed to for_each while keeping it mutable in the enclosing scope. Therefore, programmers have to declare factor not-const within main() and I was wondering if there was a way using Phoenix to add the const only locally within the "function" passed to for_each (this is done by Boost.Loccal using "constant-binding" as in `const bind& factor`).
Oh right, you can do that ...:
let(_a = ref(factor))[...]; // <-- bind as non-const reference let(_a = cref(factor))[...]; // <-- bind as const reference
Error messages are a mess currently ... but can be improved. Consider it as a bug. -- end --
Thanks. --Lorenzo
participants (4)
-
Andrzej Krzemienski -
Joel de Guzman -
Lorenzo Caminiti -
Thomas Heller