Boost logo

Boost :

Subject: Re: [boost] [yap] Review
From: Zach Laine (whatwasthataddress_at_[hidden])
Date: 2018-02-12 16:24:12

On Sun, Feb 11, 2018 at 5:53 PM, Fletcher, John P <j.p.fletcher_at_[hidden]>

> I have quite a lot to say and I may not get it all down in one go.


Hi, John. Thanks for reviewing.

> > * Implementation

> I have encountered quite a lot of the implementation as I have worked on
> the examples. I appreciate that this library makes extensive use of Boost
> Hana. There are some places in the examples where the user needs to have
> an understanding of Boost Hana to work. For example the transform example
> for arity uses Boost Hana maximum and transform. I will except
> boost::hana::tuple from the comment as it is used so much and is similar to
> std::tuple.
> One issue I have had is with the namespace. I am not clear why some code
> needs the full name e.g. boost::yap::evaluate and in other places
> evaluate will do without using being defined. It would be good if that was
> consistent.

Could you give an example? I suspect that some cases where evaluate is
used are in library details where that name is in scope, and some are used
in places where I did not want a common name like evaluate() to resolve to
a different implementation due to ADL.

> * Documentation
> I don't think that the documentation at present does full justice to the
> code. There are several things which I found very useful which are not
> much mentioned in the code.

Please be as specific as you can. This is an area that badly needs
attention, and which I have struggled with so far. Outside perspectives
are always invaluable here.

> When I saw that the examples were taken from work with proto I set out
> to make one which was missing, the mini_lambda example. I have had a lot
> of success with that while having to dig quite hard to find information.
> make_expression_function
> I have found this very useful in moveing to a functional approach. It is
> not much mentioned.

This was very easy to implement, and made near-parity with Proto a bit
easier to accomplish. It's not emphasized simply because its a utility,
and doesn't match the main use case, which is:

auto expr = /* ... */;
auto expr_2 = yap::transform(expr, my_xform);
// however many more transforms...
auto result = yap::evaluate(expr); // or an evaluating transfom...

Anything that does not fit that main use case is generally given little
time in the docs.


Thanks a gain for the review!


Boost list run by bdawes at, gregod at, cpdaniel at, john at