Re: [boost] [Hana] Formal review for Hana

Subject: Re: [boost] [Hana] Formal review for Hana
From: Paul Fultz II (pfultz2_at_[hidden])
Date: 2015-06-13 13:32:43

Here some points I see as I have been reviewing Hana. I see you have made
some
changes on github, so maybe some of this doesn't apply anymore.

- Dot shouldn't be used in names, underscores should be used instead.

- `fold` and `reverse_fold` should be preferred over `fold_right` and
  ``fold_left`. This is more familiar to C++ programmers.

- Concepts are capitalized, however, models of a concept should not be
  capitalized(such as `IntregralConstant`, `Either`, `Lazy`, `Optional`,
  `Tuple`, etc)

- IntregralConstant is very strange. In Hana, its not a concept(even though
  its capitalized), but rather a so called "data type". Furthermore, because
  of this strangeness it doesn't interoperate with other
  IntregralConstants(such as from Tick) even though all the operators are
  defined.

- The `.times` seems strange and should be a free function so it works with
  any IntregralConstant.

- In the section 'Taking control of SFINAE', seems like it could be solved a
  lot simpler and easier using `overload_linear`.

- Concepts refer to 'superclasses' these should be listed either as
  refinements or listed under the requirements section(which seem to be
  missing). It would be nicer if the concepts were documented like how they
  are at cppreference: http://en.cppreference.com/w/cpp/concept

- Concepts make no mention of minimum type requirement such as
  MoveConstructible.

- Organization of documentation could be better. Its nice showing what
  algorithms when the user views a concept, but it would be better if all
the
  algorithms could be viewed together.

- For compile-time `Iterable` sequence(which is all you support right now),
  `is_empty` can be inferred, and should be optional.

- Overall, I think the Concepts could be simplified. They seem to be too
  complicated, and it leads to many surprises which seem to not make
sense(such
  as using `Range` or `String` with `concat` or using
  `tick::integral_constant`).

- Currently, none of the algorithms are constrained, instead it uses
  `static_assert`, which I think is bad for a library that is targeting
modern
  compilers.

- It would be nice if the use of variable templates would be optional(and
not
  used internally), since without inline variables, it can lead to ODR
  violations and executable bloat.

Overall, I would like to see Hana compile on more compilers before it gets
accepted into boost(currently it doesn't even compile on my macbook).

Thanks,
Paul

--
View this message in context: http://boost.2283326.n4.nabble.com/Boost-Hana-Formal-review-for-Hana-tp4676969p4677087.html
Sent from the Boost - Dev mailing list archive at Nabble.com.