[yap] Review

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[yap] Review

Boost - Dev mailing list
I have quite a lot to say and I may not get it all down in one go.

> - Whether you believe the library should be accepted into Boost

Accept.

> - Your name

John Fletcher

> - Your knowledge of the problem domain

I have been working on codes doing similar things for a long time.  I started with FC++ and have worked with Phoenix (I did some maintenance) and a lot of other codes.  I have also worked with Hana around the time of that review.  I tend to work by trying things out and extending the examples given, which is what I have been doing with YAP.  I suppose you can categorize me as someone working at an intermediate level, attempting to make use of libraries to make results available to users at a higher level in a way which is intuitive.  As I have worked in the past with the old C++ I particularly appreciate new codes which take advantage of C++14 to make this work easier.

> You are strongly encouraged to also provide additional information:

> - What is your evaluation of the library's:
>  * Design

I like the design and have found this grow on me as I worked with the examples and saw how the pieces work together.  I have found some of the recent discussion about why things are how they are very helpful.

>  * 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.

>  * 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.  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.

I think what is needed is more examples and some contributions from users as to what help is needed to understand how to get the best use out of YAP.

 > * Tests

I have already mentioned working on a version of the proto mini lambda.

I have also built and example of the member call operator as the only example was in a more complicated example.

I have also extended the vector example to add some cases which are missing e.g.  3. * v3 and v3 * v .  I have made it use  v3*v3 to return a scalar product.

>  * Usefulness

I have found this to be useful from the examples.

> - Did you attempt to use the library? If so:

Yes

>  * Which compiler(s)?

Mainly  clang 4.0 C++14 with corresponding libc++.   I have run some examples with gcc 7.1 C++14.

This work is on ubuntu linux.
 
>  * What was the experience? Any problems?

Problems encountered have been with my own understanding and wanting to explore the boundaries.

> - How much effort did you put into your evaluation of the review?

I have spent many hours spread over several weeks.  I started before the start of the review.

I hope this is helpful.

Zach, thank you for your work and please ask if anything is not clear.

Best wishes

John Fletcher

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [yap] Review

Boost - Dev mailing list
On Sun, Feb 11, 2018 at 5:53 PM, Fletcher, John P <[hidden email]>
wrote:

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


[snip]

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.

[snip]

Thanks a gain for the review!

Zach

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost