Re: [boost] Re: [review] hash functions

From: Peter Dimov (pdimov_at_[hidden])
Date: 2005-03-10 12:48:54

Dave Harris wrote:
> In-Reply-To: <d0pe8q$tdi$1_at_[hidden]>
> daniel_at_[hidden] (Daniel James) wrote (abridged):
>> But it will be easy to make mistakes for more complicated (possibly
>> recursive) types. Especially since hash_combine is not associative or
>> transitive. I could see that leading to mistakes.
>
> I can see that it's possible to call the function with the wrong
> arguments, but that kind of problem can be addressed with
> documentation and/or by careful naming. I don't think that's worth
> distorting the
> interface and introducing side effects and spurious variables and
> inefficiency to fix it.

Strong words. "Distorting" the interface improves its usability by removing 
a potential source of mistakes. The mere presence of side effects is not, in 
itself, an argument. No "spurious" variables are introduced; naming the seed 
variable, in the cases where it's not already named, can improve 
readability. As for the inefficiency, perhaps you have the numbers to prove 
it.

>> size_t my_hash_function(int x[4]) {
>>      using namespace boost;
>>
>>      return hash_combine(
>>          hash_combine(hash_value(x[0]), hash_value(x[1])),
>>          hash_combine(hash_value(x[2]), hash_value(x[3])));
>> }
>
> I agree this is wrong. I think:
>
>      return hash_combine( hash_combine( hash_combine(
>            hash_value( x[0] ), x[1] ), x[2] ), x[3] );
>
> is reasonable and ideally would be supported.

The idea was to make the wrong version difficult to write. It is true that 
this also makes the "reasonable" version difficult to write. ;-)