[boost] Hash function review

From: John Maddock (john_at_[hidden])
Date: 2005-03-14 07:30:08

A little late in day, here we go:

>What is your evaluation of the design?

The design is basically fixed by the TR, so there's not much to review here. 
The extensions to the TR proposed by Peter Dimov, and implemented here look 
fine to me as well.

>What is your evaluation of the implementation?

Modulo the suggestions to the pointer hashing, it looks very good: I spent 
some time examining the floating point implementation, along with the 
associated test cases, and everything that could be tested looks like it is. 
I agree with those opinions expressed here that it's better to have a 
reliable implementation, that produces good hash values, than an 
artificially fast version that produces bad hash values.

I have one query regarding hashing of integer types, regarding hashing of 
long long, Daniel says:

"Also, it will need a different hash function to the other integer type
as the current one won't take account of the higher bits. I want to take
my time over things like this and get it right."

But I think this is a (mostly theoretical) issue already: type long may have 
more bits than size_t does.  I don't think this is a big issue though, it 
should effect very few platforms, and shouldn't hold up acceptance of the 
library.

>What is your evaluation of the documentation?

Also looks very good, I would like to see some more description of how the 
implementation differs from the TR (all the bits are there in the docs, it's 
just not clear which are part of the TR, and which are the extensions).

>What is your evaluation of the potential usefulness of the library?

Can't believe we haven't had this in Boost already.  It's about time.

>Did you try to use the library?  With what compiler?  Did you have any 
>problems?

I've spent some time examining and running the test cases, everything that 
should pass does so with:

gcc-3.3.2
Intel 7, 8.1 and 9Beta
VC++ 6, 7, 7.1 and 8Beta.

The tests are failing with Borland, due to a problem with Boost.Test.

>How much effort did you put into your evaluation? A glance? A quick 
>reading? In-depth study?

A couple of hours reading the code and the docs.

>Are you knowledgeable about the problem domain?

I don't claim to be any kind of expert in hashing.

>Do you think the library should be accepted as a Boost library?

Yes, please ;-)

John.