[boost] Re: Comments on functional/hash

From: Daniel James (daniel_at_[hidden])
Date: 2005-04-02 07:26:07

Peter Dimov wrote:
> First of all, nice work!

Thanks. I was planning on asking for a post-review once I'd finished the 
documentation. I rushed the headers and tests into CVS early so that 
MultiIndex can use them, and I can get regression test results - 
otherwise I won't make the 1.33 release schedule.

I mostly agree, or feel indifferent, about everything you wrote. So only 
a couple of quick responses.

> * hash/hash.hpp contains hash_range overloads for Range objects
> 
> I don't recall this being discussed during the review.

It was, but very briefly. (At least, it somehow made its way into my notes).

> This introduces a dependency on the range headers. Given that std::pair 
> is separated in its own header, this is a bit odd, especially when the 
> range headers include <utility>.
> 
> But what's more important, these overloads are unnecessary. The proper 
> protocol to obtain the hash value of an object x is hash_value( x ), not 
> hash_range( x ). Consider the case std::vector<X>. If X does not support 
> hash_value, hashing the vector will fail, even in the case where 
> hash_range could have been used for X.
> 
> If a range object is not supported by hash_value, we just need to add a 
> hash_value overload for it.
> 
> Summary: these overloads need to be removed.

Well, I'll do this, unless anyone comes up with a good reason not to.

> * boost::hash derived from std::unary_function
> 
> This introduces a completely unnecessary dependency on <functional>. 
> boost::hash only needs to define result_type and argument_type.

I thought this was required by TR1. I'll happily remove it if isn't.

Daniel