Re: [boost] Re: [review] hash functions
In-Reply-To: <d11prg$uvt$1@sea.gmane.org> daniel@calamity.org.uk (Daniel James) wrote (abridged):
Well, we did discus the type of review before it started, and nobody raised any objections.
That's partly because it sounded like the interface was fixed and known good. Where-as we are now agreed that hash_range is going to take at least one additional parameter and possibly be renamed (to hash_combine) and the return type maybe changed to void; so the interface is not fixed. Had I known that I might have objected to the fast-track, but the only way to know it would be to actually review the proposal. Maybe if we are in a hurry we should delete hash_range and hash_combine from the initial official interface, while still using them for the implementation. That would enable the containers to move forward.
On a related note, I am going to be requesting a review for the unordered associative containers soon. It has been suggested that since they are based on TR1 they should have a shorter than normal review (but longer than this one). So I guess you think it should have a full length review?
I have no idea. I am less interested in the containers than in the hashing. I don't think I've ever implemented a hashing container in C++. I don't currently plan to contribute to the containers review. -- Dave Harris, Nottingham, UK
Dave Harris wrote:
In-Reply-To: <d11prg$uvt$1@sea.gmane.org> daniel@calamity.org.uk (Daniel James) wrote (abridged):
Well, we did discus the type of review before it started, and nobody raised any objections.
That's partly because it sounded like the interface was fixed and known good. Where-as we are now agreed that hash_range is going to take at least one additional parameter and possibly be renamed (to hash_combine) and the return type maybe changed to void; so the interface is not fixed.
No, we are not. ;-) I still support the original proposal, with the following additions: 1. Bias hash_value(v) in hash_combine to avoid the zero trap. 2. Add a hash_range overload that takes a seed. (2) can take the following forms: 2a. void hash_range( size_t & seed, It first, It last ); 2b. size_t hash_range( size_t seed, It first, It last ); 2c. size_t hash_range( It first, It last, size_t seed ); which are listed in order of my preference. The version with a default argument is equivalent to the current proposal with (2c) added. Rationale: I prefer the seed as a first argument for consistency with hash_combine. I prefer (2a) to emphasize the fact that this overload is intended to be used when you already have a 'seed' variable that accumulates the hash value computed so far. Rationale for (1): I think that it is better to fix the zero trap in hash_combine, because this does not depend on users remembering to initialize the seed to default_seed, instead of zero.
participants (2)
-
brangdon＠cix.compulink.co.uk -
Peter Dimov