Boost logo

Boost :

Subject: Re: [boost] [review][Fit] Review of Fit starts today
From: Jason Rice (ricejasonf_at_[hidden])
Date: 2017-09-19 02:48:20


Please consider this review for the proposed Boost.Fit library.

I recommend the review manager ACCEPT this library for inclusion in Boost
without condition.

# What is your evaluation of the design?
  - Making high order functions easier and more concise is a good idea.
  - I like the point-free programming features.
  - Some of the names, conventions, and errors would take
    any kind of user some getting used to.
  - A version of `alias_value` that works in a generic context where the
    type might not be an alias would be more useful I think.

# What is your evaluation of the implementation?
  - The code is consistent across implementations and the files are pretty
flat
  - Each function can be included independently which is nice.
  - It appears that there is some effort towards making errors easier to
read.
    For instance, I called `fit::partial` with too many arguments and the
error
    was "error: no member named 'fit_function_param_limit'"
  - There is some readability issues due to compiler compatibility support
and
    specific compiler error workarounds.
  - I noticed `fit::decay` is not used in the tests at all. It gets
included in
    `example.h` but is never used.

# What is your evaluation of the documentation?
  - I like that the documentation is easy to read in the comments of each
file.
  - Some of the wording in the descriptions weren't very helpful for me
personally,
    but the "Semantics" section seemed to always get the point of the
function across.

# What is your evaluation of the potential usefulness of the library?
  - It seems parts of the library might not be that useful if you have
    constexpr lambdas, but I believe there are still features that enable
more
    concise coding even against latest C++17 features.
  - I will probably use this library now that I am familiar with it.

# Did you try to use the library? With which compiler(s)? Did you have any
problems?
  - I made a couple of my own examples.
  - I used a fairly recent fork of clang trunk with c++1z.
  - BOOST_FIT_RETURNS *crashed the compiler* when I was trying to construct
an std::array
    with some pack expansion. It looked like it crashed the parser.

# How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?
  - I spent a couple of hours looking at the docs in the source and making
my own test
    programs to see how it could be used.

# Are you knowledgeable about the problem domain?
  - I've used Boost.Hana a lot with its "functional" features.
  - I've done a lot of point free programming with RamdaJS in Javascript

# Were the concerns from the March 2016 review of Fit addressed?
  N/A

For what it's worth here is a link to some of my attempts to sample the
library:
https://gist.github.com/ricejasonf/9bfd233b3bc282eae9ecb8bc3198d948

Thank You,

Jason Rice


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk