Re: [boost] [local] Review

Subject: Re: [boost] [local] Review
From: Thomas Heller (thom.heller_at_[hidden])
Date: 2011-11-19 12:15:50

On 11/18/2011 09:37 PM, Lorenzo Caminiti wrote:
> On Fri, Nov 18, 2011 at 8:35 PM, Thomas Heller
> <thom.heller_at_[hidden]>  wrote:
>> On 11/18/2011 04:44 PM, Lorenzo Caminiti wrote:
>>> On Fri, Nov 18, 2011 at 1:49 PM, Thomas Heller
>>> <thom.heller_at_[hidden]>    wrote:
>>>> Additionally, despite the introductory motivating statement ("Local
>>>> functions are a form of information hiding and are useful for dividing
>>>> procedural tasks into subtasks which are only meaningful locally,
>>>> avoiding
>>>> cluttering other parts of the program with functions, variables, etc
>>>> unrelated to those parts.") I can not see that this library solves this
>>>
>>> Why Boost.Local does not solve this appropriately? What do you mean by
>>> "appropriately" in this context?
>>
>> It tries do make it easier for users to define local functions in order to
>> make their lifes easier so they don't have to clutter their code with local
>> functions. This is what i fail to see:
>
> Sorry, I don't understand this sentence. Can you please re-word it?
>
>> 1) The macros do not even closely resemble any kind of function declarations
>> in C++
>
> Many have expressed different opinion appreciating instead the syntax.
> For example (but also other email thread on Boost and reviews):
>
> That said, I understand how this is clearly a subjective point and I
> respect your opinion.
>
>> 2) If you just want to write a simple local function, the epi- and prologue
>> is longer than the actual function body (this is true for almost all your
>
> A part form the two macro names, you don't have more tokens than the
> result type, parameter declarations, bind declarations, and local
> function name. IMO, these are all elements that syntactically should
> be part of the local function declaration regardless of the length of
> the body because they describe the local function interface and its
> bindings.
>
>> examples). And once the function body exceeds that limit I wonder why it
>> should have been a local function to begin with.
>
> Others have mentioned that it is fine for them to have long functions
> and local functions especially in FP:
>
> -- from: http://lists.boost.org/Archives/boost/2011/02/176653.php --
>> In functional programming, there is no difference between defining lambdas
>> and functions (except the former cannot be recursive), and it is not viewed
>> as bad practice to have functions spanning a couple hundred lines.
> -- end --
>
> But of course I understand if you prefer to structure your code differently.
>
>>>> appropriately. I personally think that exposing macros as the interface
>>>> to
>>>> program with the library does not lead to a nice and clean design of ones
>>>> code.
>>>> Boost.Local is very different from other heavily macro based libraries
>>>> like
>>>> Boost.Parameter. It is different, because I would consider Boost.Local
>>>> not
>>>> as a infrastructure library but as a library for end users. This being
>>>> said,
>>>> i would not like to see application code cluttered with overly verbose
>>>> calls
>>>> to macros.
>>>> As boost is trying to not only push the boundaries of C++ it should also
>>>> try
>>>> to set examples of best C++ practice. I would not consider Macro calls to
>>>> interface with a library not good practice.
>>>
>>> I understand. I disagree but I understand your point. I disagree
>>> because the use of macros in this context saves the user from writing
>>> verbose boiler-plate code.
>>
>> What kind of boilerplate code?
>> Defining a global function? Most likely no.
>> Defining a local functor? Yeah, probably, but do we really need that?
>
> Yes, the boiler-plate code to define the local functor. And yes, you
> need that to implement the local function... Sorry but I'm missing
> something here... Can you please explain what you mean better with the
> two question marks above?

The question is about what I my point of criticism is all about:
Why do we need a macro to define a function, that adds no more true 
value other than that it was defined in function scope?

>> Defining a global functor? This is not possible with Local, so the answer
>> would be "no" here.
>>
>>> IMO, it is similar to Boost.ScopeExit: Instead of providing the
>>> SCOPE_EXIT macros we could have said that the user writes the code to
>>> bind the variables deducing their types and it programs the local
>>> class with the exit code in the desstructor all of that by hand.
>>> However, using macros saves the user to write all of that and as I
>>> user I personally appreciate that. It is true that there is a trade
>>> off between the readability cost of using macros and the benefit of
>>> not having to write boiler-plate code. I find that trade off
>>> acceptable for libraries like Boost.SopeExit or even ForEach and (of
>>> course ;) ) also for my library so I don't have to write the
>>> boiler-plate code for the binding and the casting to pass the local
>>> class as a template parameter.
>>
>> Well, I didn't say that I think ScopeExit is good. In fact, I never saw the
>
> Can you please clarify? Would you make to ScopeExit the same
> criticisms you are making to Boost.Local syntax and its macro usage or
> not? // (1)

Yes, I would make the criticism to ScopeExit.

>> need to use it. As another reviewer noted, i'd rather encapsulate that in a
>> class to logically divide that from the actual code.
>> For ForEach the situation is completely different. Apart from having it to
>> spell all caps, it looks exactly like a range based for each loop should
>> look like. Boost.Local's Macros (yes, plural!) however, have nothing in
>> common with the way you declare functions in C++.
>
> Again, I understand that you personally view the syntax as not
> natural. (I am honestly sorry to hear that because I truly spent a lot
> of effort in trying to make the syntax as natural as possible based on
> everyone's input but I don't pretend to be able to make everyone happy
> so it's ok.)

Look, I am not trying to make you feel bad. I know that you put a lot of 
effort into making this library. If you remember, I wasn't happy about 
it from the start on, and i said so. As did others.

>>>> The default parameter syntax is confusing, it looks like it is a
>>>> completely
>>>> new parameter, and not belonging to the previous argument. This gets
>>>> confusing once you have more than one parameter with a default parameter.
>>>> The WITH_DEFAULT macro should be the default, as it clearly states the
>>>> relationship of the default value to the parameter.
>>>
>>> Others have made the same comments (with you a total of 3 people).
>>> However, others have made the opposite comment (2 liked default, 5+
>>> didn't say anything about it). In my experience, this quickly becomes
>>> a non-issue after the 1st week that you start using the library.
>>
>> Right, i have to learn a new syntax in order to learn how to define local
>
> That is true for Boost.Lambda, Boost.Phenix, and even for C++11
> (because most programmers are now only familiar with C++03). All these
> tools require you to learn a new syntax as Boost.Local does. So what
> is your point?

The point is that the FP libraries offer additional value

>
>> functions. I note this in order to support the argument that Boost.Local
>> does not solve the issues discussed above.
>
> Let me clarify that Boost.Local goal is to support local functions
> with bindings and statement syntax for the function definition.
> Boost.Local never tried to solve this problem without introducing a
> new syntax for the function declaration (in fact earlier version of
> the library had an even more exotic syntax for declaring the local
> function).

Right, looks like i just miss the point.

>>>> Additionally as binding is probably the right term to use here, it might
>>>> confuse people with the already existing solutions of the various bind
>>>> implementations.
>>>
>>> I don't think so... here bind is clearly used in a different context.
>>
>> Yes that is what i meant. It is a overloading of terms.
>
> Why would it be confusing? You spent a few hours reading the docs, do
> you honestly find it confusing? If so, do you think that a name
> different than bind should be used? (I can use any name we can think
> of and agree on.) For example, would "capture" be more clear:
>
> int x = 0;
> void BOOST_LOCAL_FUNCTION_PARAMS(const capture&  x) {
>      ... // some long set of instruction ;)
> } BOOST_LOCAL_FUNCTION_NAME(f)
> f();

I never said it is confusing to me. I said to users. The comment above 
was trying to avoid overloading of terms. "bind" certainly has some 
history in boost and C++. That is all. You could argue that bind your 
context does something similar.

>>>> Local Blocks:
>>>> I can't see the value in that one. The example in the documentation
>>>> doesn't
>>>> help either, as those things are easily detected by any modern compiler,
>>>> with the appropriate warning levels (ok, not in the assert case, but for
>>>> other boolean contexts at least gcc does).
>>>
>>> I am sure there are lots of people that will never use a local blocks.
>>> That said, I personally needed to program (to ensure the
>>> constant-correctness requirement of assertions in Contract
>>> Programming):
>>>
>>> int x = 0;
>>> const {
>>>      assert( x == 0 );
>>> }
>>>
>>> You can do that with local blocks:
>>>
>>> int x = 0;
>>> BOOST_LOCAL_BLOCK(const bind x) {
>>>      assert( x == 0 );
>>> } BOOST_LOCAL_BLOCK_END
>>>
>>> As we said, using lambdas or other Boost libraries there are other
>>> ways to do that but in my case I needed to keep the `assertion( x == 0
>>> )`;` unchanged (including the variable name `x`) and these other ways
>>> (including C++11 lambdas) don't allow for that.
>>>
>>> More in general, local blocks allow you to specify the sematic that a
>>> variable should be thread as const by the next bunch of instruction. I
>>> personally find that useful, others have made the same comment
>>> (Vicente, Pierre, Jeff, and another couple of people-- see their
>>> review to Boost.Local).
>>
>> Sorry, I just don't get it ... why does it need to be surrounded by a macro?
>
> Because I could not find any other way to declare the local function
> using statement syntax for its definition without using macros. Does
> this make sense?

No. I was aiming at: Why do you need a local function at all, when all 
you want is a new scope where you don't have access to the outer 
variables? That sounds awfully familiar to what a normal function is.

>> I can't fully appreciate the value here. Not because of other existing
>> libraries, but because normal functions are way more expressive and clear
>> than these macros can ever be.
>
> But "normal" functions cannot be declared at local scope so they are
> not sufficient for the problem domain of this library.

This is exactly what I am trying to say. It looks to me that local is 
trying to solve a non-issue (at least for me).
I argue that local functions are not needed in C++.