[boost] Re: Lambda Library.

From: Dave Abrahams (abrahams_at_[hidden])
Date: 2000-03-12 01:49:41

on 3/10/00 11:46 AM, Gary Powell at Gary.Powell_at_[hidden] wrote:

> What we'd like is comments on code review, design review, usefulness,
> forward direction suggestions, etc. (I'd ask for peer review, but I'm not
> sure I qualify to be a peer after reading all your posts.) (Shameless
> flattery.)

Having looked at the documentation, but not the code, I think this is
beautiful work. If I thought this could have been carried to a conclusion
anywhere nearly this satisfactory I would have pursued it myself months ago.

The things I can find to criticize so far are:

1. It's a little disturbing that you can write 'obvious' things which
compile and 'should' work, but fail only at runtime, e.g.
> for_each(a.begin(),a.end(), cout << ", " << free1); //valid, but wrong
> for_each(a.begin(),a.end(), cout << constant(", ") << free1); //ok

Incidentally, wy doesn't the first one work, if your example "free1+5" does
work? How does the user know when she must write constant()?

2. Naming: It's best to avoid abbreviations, so I don't like 'var'. I think
'argument' would be much more descriptive than 'free'. 'create_ptr' doesn't
create a pointer, it creates an object on the heap (maybe 'new_object'?)

3. Documentation. You've done a remarkably thorough job of documenting this,
especially for an alpha release, but:
 a. I'd like to see you qualify all names with their respective namespaces.
That will give a much better picture of how it would look in real,
best-practices C++ code.
 b. Occasionally, you reveal implementation details which only seem to
confuse the issues, without giving the user additional expressive power:

> int i = 0; 
> int j;
> for_each(a.begin(),a.end(), (var(j) = free1, free1 = var(i), var(i) =
> var(j)));  
>
> The delayed variables and constants can be defined outside the labmda
> expression as well:
> int i = 0; int j;
> var_type<int>::type vi(var(i)), vj(var(j));
> for_each(a.begin(),a.end(), (vj = free1, free1 = vi, vi = vj));
> 
> constant_type<int>::type five(constant(5));
> for_each(a.begin(),a.end(), (vi = five, free1 = vi * constant(6) ));

Why would anyone want to write it that way? It's much harder to read than
the original.

Your control-flow examples could benefit from some well-chosen line breaks:

Control flow examples:
for_each(a.begin(),a.end(),
  if_then(free1 > 5,
          free1 = 5) );

for_each(a.begin(),a.end(),
   if_then_else(free1 > 0,
       free1 = bind(sqr, free1),
       0) ); 

By the way, this one isn't a very compelling case for if_then_else, is it?

> int i;
> var_type<int>::type vi(var(i));
> for_each(a.begin(),a.end(), (vi = 0,
> while_loop(vi < free1, cout << constant("-"), ++vi ) ) );

Isn't this clearer?

int i;
for_each(a.begin(),a.end(),
(
    var(i) = 0, 
    while_loop(var(i) < free1,
        cout << constant("-"),
        ++var(i)
    )
)
); 

Incidentally, aren't there some parens missing around the suboordinate
clauses of the while_loop?

for_each(a.begin(),a.end(),
    for_loop(vi = free1, vi > 0, --vi,
        cout << (free1 - vi)
    )
);

int c[10][5];
for_each(c, c+5,
    for_loop(vi = 0, vi < 10, ++vi,
        cout << free1[vi] << ", "
    )
);

4. I would think a lambda version of ?: would be more important than
if_then_else... how 'bout it?