On 5 Jun 2015 at 16:57, Glen Fernandes wrote:
- Whether you believe the library should be accepted into Boost
I vote unconditional acceptance.
- Your name
Niall Douglas.
- Your knowledge of the problem domain.
Very little. Moreover, the type of C++ programming used in its implementation I find a major chore to write, and a lot of it is frankly beyond me (as you'll see in my shortly to presented optimally lightweight monad<T>). I am particularly hoping that Hana will eventually free me of ever having to do it by hand again in the future once Visual Studio can compile Hana.
You are strongly encouraged to also provide additional information: - What is your evaluation of the library's: * Design
There is a still a bit to go to match STL naming conventions, though it has improved enormously over before. For example, the STL uses empty(), not is_empty(). is_empty() in the STL means something different. I'd also *hugely* prefer if Hana matched, name for name, the name choices in Ranges v3. For example, it's group() in Hana, but group_by() in Ranges v3. That would lessen the cognitive load for people using both together - which I suspect in the longer term will be many if not most. It also would increase the chances of Hana entering the standard C++ library as a compile-time version of Ranges. It also provides a way of telling the naming bike shedders to sod off, as whatever Eric has chosen is what you'll choose period. I also think all the Concepts need to match naming with Eric's, and eventually in the future that both libraries use identical Concept implementations (which I assume would then be upgraded with Concepts Lite on supporting compilers). I'd suggest therefore breaking the Concepts stuff out into a modular part easily replaceable in the future with a new library shared by Hana and Ranges.
* Implementation
I won't comment on this as I am not really qualified to say. It looks fine.
* Documentation
I agree with other reviewers that the code examples need a hana:: namespace qualifier before all uses of hana stuff. I quite like the doxygen prettification, maybe matching the Boost colour scheme more? One problem is slow page load times on IE, especially the first page, but also every click in the left hand table of contents. I also see no graphs displaying either in IE or Chrome. BTW for my lightweight monad<T> I have some python which counts x64 ops generated by a code example and gets the CI commit to fail if limits are exceeded, if you're interested for the benchmarking and/or making hard promises about runtime costs.
* Tests
Tests should be capable of using BOOST_CHECK and BOOST_REQUIRE as well as static_assert. It should be switchable. You can then feed the Boost.Test XML results into the regression test tooling.
* Usefulness
Very useful.
- Did you attempt to use the library? If so: * Which compiler(s)? * What was the experience? Any problems?
None.
- How much effort did you put into your evaluation of the review?
I've been reviewing the project for over a year now regularly, and have been known to write Louis private emails with ideas :) Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/