Re: [boost] [Hana] Informal review request

Subject: Re: [boost] [Hana] Informal review request
From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2015-03-05 14:58:12

AMDG

On 03/05/2015 12:02 PM, Louis Dionne wrote:
> Steven Watanabe <watanabesj <at> gmail.com> writes:
> 
>>
>> I've just glanced over the main page of the documentation
>> and I had a few comments:
>>
>> - _tuple<Person, Car, City>
>>   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<Types...>` 
> 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