Re: [boost] [Hana] Informal review request

Steven Watanabe writes:

AMDG

I've just glanced over the main page of the documentation and I had a few comments:

- _tuple Historically names beginning with _ are used for placeholders. i.e. in Boost.Bind/Lambda/Phoenix/Parameter. Is there a good reason not to call it tuple?

It is because of other constructs in Hana like `tuple_t` which are variable templates and whose type is not specified. I have not found a good naming convention for variable templates and types yet. I'll sketch out the current state of affairs, and perhaps someone can suggest a convention that would work well. Hana currently proposes the following: _tuple : The type of a tuple containing objects of type T... make_tuple(x...): Create a tuple of type _tuple...> tuple(x...): Currently equivalent to make_tuple(x...), but deprecated because I'd like to use it in place of _tuple. Tuple: The generalized data type of tuples. tuple_t: A variable template returning a tuple containing types. This object has an _unspecified type_, but its data type is Tuple. In other words, this behaves as a tuple, but it does not have the same representation. It's for optimization purposes. Where it becomes really tricky is that the type of this object is dependent, so it can't be used for pattern matching. However, it is currently specified as inheriting the _tuple_t type, which allows one to pattern match in overloaded functions. tuple_c: A variable template returning a tuple containing integral constants. This returns an object of type _tuple_c, which is not the same as _tuple. It behaves like a tuple, but has a different representation. For optimization purposes too. Other components follow similar conventions. For example, _range: The type of a range containing IntegralConstants of data type T spanning the given interval. Range: The generalized data type of _range<...>. make_range(from, to): Create a _range. range(from, to): Equivalent to make_range(from, to), but deprecated because I'd like it to replace `_range`. range_c: Shortcut to create a range of IntegralConstants. This is similar to vector_c in the MPL. And so it goes. Writing this, perhaps it would make sense to simply replace `_range` by `range`, `_tuple` by `tuple` and etc.

- // BOOST_HANA_RUNTIME_CHECK is equivalent to assert In that case, what is the point of using it instead of assert? I think you should probably avoid using these constructs in the user documentation of other parts of the library. It's just a distraction, so it's better to rely on standard components, which users probably already understand.

The problem is that `assert` is not define when NDEBUG is defined, but I want my tests to fire up all the time. Since these examples are Doxygen'd out of actual source files, the macros end up in the documentation. Basically, If I could redefine `assert` to be `BOOST_HANA_RUNTIME_CHECK` in those test files, I'd be really happy. Would it be against good software craftsmanship to write ... include all you need ... #ifndef assert # define assert(...) BOOST_HANA_RUNTIME_CHECK(__VA_ARGS__) #endif

- Is there a reason for using length instead of size, which would be more idiomatic in C++?

Oversight caused by personal preference. I guess I should change that.

- Amphibian algorithms!!!??? Okay, I guess the name is kind of clever. I can't say that I really like it, though.

If you have anything better to propose, I'm all ears.

- foldl/foldr. I really hate these names. I'd prefer fold/reverse_fold, but even fold_left/fold_right would be better.

I personnally hate reverse_fold, since I don't see how it is a "reverse" fold. It's just a fold with different _associativity_ properties. As for fold_right and fold_left, I guess it comes down to personal preference. I opened a "poll" on GitHub to settle this. See [1].

- count is the equivalent of std::count_if, not std::count. - find/lookup should be find_if/find

You're right. It's sometimes hard to set aside personal preferences for the greater good, but I'll do it :-).

- In general I would expect any algorithm that takes a single element for comparison to have both f and f_if forms.

It is possible that a structure can do lookup by equality but not by any predicate, so it wouldn't be able to provide both. But do you have an example in mind of a place where there's an *_if variant missing in Hana? It would almost surely be an oversight.

- sort(predicate): shouldn't this be sort(sequence)?

Typo; thanks for reporting. Louis [1]: https://github.com/ldionne/hana/issues/18