On 5 Jun 2015 at 16:57, Glen Fernandes wrote:
[...]
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.
Actually, a lot of the names are very close. I just checked Eric's range library and there are a __lot__ of resemblances. I guess there was a common inspiration (Haskell?) or a lot of luck. I'll try to converge even more towards his naming.
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.
lol
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.
That's a noble goal, but it is completely impossible without some redesign of Range-v3's concepts. Range-v3 is based on runtime concepts like ForwardRange, which is itself based on ForwardIterator. There is just no way heterogeneous containers can be made to model these things, even if we were to have an iterator-based design like Fusion. The problem here lies in compile-time vs runtime. But I could be wrong, and maybe Eric can tell us more about that.
* 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.
I can see the charts on Google Chrome. Perhaps you reloaded the page a ton of times? If so, there's a limit on the number of queries one can do to fetch the data sets from GitHub. After something like 50 reloads, you have to wait one hour. It shouldn't be a problem for most people, though.
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.
I might be interested; where's that code?
* 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.
What about tests that fail/pass at compile-time? How does the MPL handle that? Also, passing a BOOST_CHECK assertion does not necessarily mean anything for Hana, since sometimes what we're checking is not only that something is true, but that something is true __at compile-time__. How is the MPL integrated with the regression test tooling? I think that is closer to what Hana might need?
* 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 :)
Thanks a lot for your review and comments during the past year; you've been providing me with invaluable feedback and I appreciate that. Regards, Louis -- View this message in context: http://boost.2283326.n4.nabble.com/Re-Boost-announce-Boost-Hana-Formal-revie... Sent from the Boost - Dev mailing list archive at Nabble.com.