Re: [boost] [Fit] Formal review / Kris

On Saturday, March 19, 2016 2:26 PM, Krzysztof Jusiak 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: http://lists.boost.org/mailman/listinfo.cgi/boost