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.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk