Re: [boost] [local] Review request

Subject: Re: [boost] [local] Review request
From: Mostafa (mostafa_working_away_at_[hidden])
Date: 2011-05-15 00:42:37

On Sat, 14 May 2011 18:24:36 -0700, Lorenzo Caminiti  
<lorcaminiti_at_[hidden]> wrote:

> On Sat, May 14, 2011 at 9:06 PM, Mostafa  
> <mostafa_working_away_at_[hidden]> wrote:
>> On Sat, 14 May 2011 12:38:50 -0700, Lorenzo Caminiti  
>> <lorcaminiti_at_[hidden]>
>> I just took a quick glance at the documentation to get an understanding  
>> of
>> the library, and I have a suggestion/comment:
>>
>> 1) I suggest adding:
>>
>> #ifdef ENABLE_BOOST_LOCAL_VARIADIC_WITH_DEFAULT
>> #define WITH_DEFAULT , default
>> #endif
>>
>> #ifdef ENABLE_BOOST_LOCAL_SEQUENCING_WITH_DEFAULT
>> #define WITH_DEFAULT ) default
>> #endif
>>
>> to the library.  I think it makes client code more readable if they  
>> define
>> ENABLE_BOOST_LOCAL_VARIADIC_WITH_DEFAULT or its variant rather than just
>> defining WITH_DEFAULT.
>
> Something similar (at least for the variadic syntax) is suggest in one
> of the docs examples -- see last example here:
> http://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Tutorial.html#boost_local.Tutorial.default_parameters
>
> However, Boost macro naming conventions will require this macro to be
> named BOOST_LOCAL_WITH_DEFAULT if the macro were to be added to
> Boost.Local. IMO, that name is too long defeating the increased
> readability benefit. Therefore, I'd leave it up to programmers to
> #define WITH_DEFAULT if they wish to do so as suggested by the above
> doc example.

Yes, it was from the documentation where I originally got my motivation  
from.  Just to be clear, I'm not suggesting replacing WITH_DEFAULT with  
BOOST_LOCAL_WITH_DEFAULT, or with my equivalents.  Rather I was suggesting  
a "standard" switch be available which, if defined by the client, would  
enable the WITH_DEFAULT macro.  As to the name being too long, I  
envisioned it (BOOST_LOCAL_WITH_DEFAULT or its equivalents) to be defined  
only *once* by the clients in a common header file, and then they would  
use the subsequently available WITH_DEFAULT through out the rest of their  
code.

I guess I'm just being super lazy, because if I were going to define  
WITH_DEFAULT myself, then I would have to explicitly add a comment that  
its for use with Boost.Local, else it would look like a very strangely  
defined macro that would require a grep to figure out what it's used for.   
(I like self documenting code, hence the suggested names.)

>> 2) In you're email you mentioned that Boost.Local functions can be used  
>> with
>> stl algorithms, is this noted in the web documentation?
>
> Yes, it is mentioned in a few places starting at the very beginning of
> the Introduction section (3rd bullet point from the top):
> http://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/index.html#boost_local.Introduction
>
> Here is a sketch of how Boost.Local implements passing local classes
> as template parameters (normally not allowed in ISO C++):
> http://svn.boost.org/svn/boost/sandbox/local/libs/local/doc/html/boost_local/Implementation.html#boost_local.Implementation.local_classes_as_template_parameters

Thanks, I don't how I missed that first mention of it.