Boost logo

Boost :

Subject: Re: [boost] [Fit] Formal review / Kris
From: paul Fultz (pfultz2_at_[hidden])
Date: 2016-03-19 20:45:45

> On Saturday, March 19, 2016 2:26 PM, Krzysztof Jusiak <krzysztof_at_[hidden]> wrote:
> > Hi,
> Firstly, sorry for submitting my review so late.
>> - Whether you believe the library should be accepted into Boost
> Yes. Although the submission is not perfect, I prefer boolean logic, and,
> therefore Fit a 1 for me.
>> - Your name
> Krzysztof Jusiak
>> - Your knowledge of the problem domain.
> I have been implementing similar solutions myself when experimenting
> with lambdas. I have also checked out functional part of Hana library.
>> You are strongly encouraged to also provide additional information:
>> - What is your evaluation of the library's:
>> * Design
> IMHO design is decent. Functionality is loosely coupled and really easy to
> follow.
> There are a few inconsistencies here and there but there were pointed out
> already
> and they don't have a huge impact on the overall solution.
>> * Implementation
> Quite a lot of macros but most of these are due to lack of language
> features or compilers bugs.
> Besides that, it looks alright. Moreover, I do agree with most issues
> pointed out already.
>> * Documentation
> Motivation could have been much better. Examples showing how much pain might
> be saved using Fit in comparison to writing the same things by hand would
> be great.
> Moreover, ready to copy examples would simplify testing the library.
> Personally, I would also add disassembled code for these examples showing
> how
> well-optimized code Fit can produce. Maybe, it's worth to add try-it-online

> functionality too?

Thats a good idea. I'll try show some disassembly to show how well the
optimizer does. I like the idea of having a try-it-online, however, I would
like a way to better handle versioning and offline documentation. I am sure
there is a nice solution to this I just haven't time to figure it out yet.

>> * Tests
> Tests are short and easy to follow. I can't see coverage but it seems like
> most
> things are tested. Travis addition with each push make me confident that
> the library
> is always in a good shape.
> I would like to see performance tests though for both compilation times and

> run times.

Yes, I have run a couple of local tests to test performance. I could pull
together some report to put in the documentation. Ideally. it would be good to
have a comparison with other libraries as well. I haven't compared it with
Hana yet, either, which would be a good idea. However, I do know that the
implementation of `fit::conditional` is much faster than the implemenation
used for `overload_linearly` in Hana, but I haven't done a direct comparison.
I'll try to add something like that. I think it will be useful.
>> * Usefulness
> IMHO Fit is defo a library to have in Boost for modern C++ development.
> I find the solutions provided by Fit very useful as I have been writing
> similar functionality myself. Nevertheless, documentation needs improving
> (as it was stated already) in order to state the purpose more clearly with
> clean examples. I have been worrying that there will be a clash with Hana.
> Having 2 modern libraries providing similar functionality would be
> confusing,
> however, my concerns were already clarified by Louis.
>> - Did you attempt to use the library? If so:
>> * Which compiler(s)
> Clang 3.7/3.8/HEAD, MSVC 2015
>> * What was the experience? Any problems?
> No issues.
>> - How much effort did you put into your evaluation of the review?
> A few hours for the review, however, I have been looking and trying

> Fit for a while.

Thanks Kris for the review and feedback.

> Cheers,
> Kris
> _______________________________________________
> Unsubscribe & other changes:

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