Re: [boost] [Hana] Informal review request

AMDG On 03/05/2015 12:02 PM, Louis Dionne wrote:

Steven Watanabe writes:

...

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.

I see. You have too many similar names. I'm not fond of it, but I can't think of anything better off the top of my head.

...

- // BOOST_HANA_RUNTIME_CHECK is equivalent to assert In that case, what is the point of using it instead of assert? <snip>

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

It won't work. assert is still defined with NDEBUG. It's just defined to do nothing. Personally, I'd just use assert and make sure that NDEBUG is not defined. It isn't really important though, and I don't think it's a good idea to put ugly workarounds in examples, since the whole point of examples is to be relatively easy to understand.

...

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

I personally 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].

I see. I think of the algorithm as starting with the state and merging in successive elements. foldl starts at the beginning of the sequence and works it way to the end, while foldr starts at the end and works its way to the beginning. It sounds like the difference is that you're thinking of fold as a single mathematical operation, instead.

...

- 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 :-).

I wouldn't really object if you only provided predicate forms. The problem is that some algorithms have only a predicate form, while others have both predicate and value forms. If you do provide both, than you need a consistent naming convention, and while f/f_if aren't the greatest names ever, they will be familar to most C++ programmers.

...

- 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.

That seems like a serious limitation. Logically find(s, x) should be equivalent to find(s, [](auto y) { return x == y } ) Also, your Searchable seems to be based on predicates, so I'm not sure how such a structure would work with your library.

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.

replace only seems to have a predicate form. In Christ, Steven Watanabe