Boost logo

Boost :

From: Jamie Allsop (ja11sop_at_[hidden])
Date: 2007-12-18 22:04:07


First apologies for the belated review and its brevity - I really had
hoped I'd have time to be more thorough, but here goes.

First some comments on the docs (nits/suggestions really). The
documentation is really very good.

---
Introduction
2nd para -
"For a hash table you need an equality function and a hash function for 
the key." I got the impression you were contrasting this with 
requirements of the ordered containers requiring less than comparable 
values. If that is so then you might want to use something like, "In 
contrast a hash table ..." of "For a hash table you _only_ need ..." as 
you are using this to point out an advantage of hash tables.
3rd para -
"So the C++ Standard Library Technical Report introduced ..." I think 
needs re-worded slightly. 'So' presumably refers to the advantages of 
the hashed containers. In this case it is abrupt and not very specific. 
Perhaps you could relate the start of the sentence to the advantages you 
oultine more directly. For example, "With this in mind ...", or simply 
start, "The C++ Library Technical Report has therefore introduced ..."
Last para -
"There are other differences, which will be detailed later."
If there are one or two specific places where these are enumerated it 
would be nice to directly reference the sections.
---
Data Structure
2nd para -
A reference to the "Equality Predicates and Hash Functions" would be 
nice, if only to indicate to the reader that there is more detailed 
information. You could perhaps hyperlink "hash function" to that section.
3rd para -
"If at a later date the container wants to find an element in the 
container it just has to apply the same process to the element's key to 
discover which bucket it is in."
This rather long sentence could do with one or more commas to make 
reading it easier for example,
"If at a later date the container wants to find an element in the 
container, it just has to apply the same process to the element's key to 
discover which bucket it is in."
Better yet, try to split it into two sentences (if that is possible).
4th para -
/comparison/comparisons/
---
Controlling the number of buckets
1st para -
"...performance to get worse." Should this be "...performance to 
worsen/degrade."
2nd para -
"First you can specify the minimum number of buckets in the constructor, 
and later, by calling rehash."
Ok, so you mean that you can specify a minimum number of buckets when 
you construct your container, after construction you can also influence 
the bucket count by calling rehash? The "... ,and later, ..." threw me 
the first time I read it.
3rd para -
"The other method is the max_load_factor member function."
Ok, now I am confused... are there three methods? Or are the two methods
1 - specify the minimum number of buckets in the constructor
2 - the max_load_factor member function
...and calling rehash is relevant to both. I think this and the previous 
paragraph need to be made more coherent.
6th para -
"Note: rehash's argument is the number of buckets..."
Ok, now I think I am seeing what you want to say in the 2nd paragraph -- 
the number of buckets can be specified either during construction or 
when rehash is called?
"The +  1 is required because the container is allowed to resize when 
the load factor is equal to the maximum load factor."
For example? It's late here and I'm not sure what you mean (but I guess 
it will be obvious in the morning :) )
---
Equality Predicates and Hash Functions
This section has been well discussed in previous posts. However on 
reading it I thought that an earlier explicit reference to Boost.Hash 
might be appropriate, spelling out the relationship between the two 
libraries.
---
One thing that would be nice are some performance details, perhaps 
comparisons against some other library implementations using the same 
has function. Just a thought.
---
The rest of the docs go into a variety of details about specific topics 
and is all very good.
Ion Gaztañaga wrote:
> * What is your evaluation of the design?
Well thought out and closely tracking TR1.
> * What is your evaluation of the implementation?
I haven't really looked at it - if I get a chance I will do so.
> * What is your evaluation of the documentation?
Excellent - see above.
> * What is your evaluation of the potential usefulness of the library?
We will be looking to use this library in critical areas of our codebase 
where performance is critical
> * Did you try to use the library? With what compiler?
Not yet.
>    Did you have any problems?
N/A
> * How much effort did you put into your evaluation?
>    A glance? A quick reading? In-depth study?
I read the docs a few times and looked at other references, plus 
reviewed the previous reviewers comments.
> * Are you knowledgeable about the problem domain?
So so. I haven't done a lot of work with hash containers before (I 
haven't needed to). I DO need to make heavy use of them now so this 
library is very timely. I've unfortunately not been able to get time to 
replace our existing implementation with this one and do a performance 
comparison.
> 
> And finally, every review should answer this question:
> 
> * Do you think the library should be accepted as a Boost library?
>    Be sure to say this explicitly so that your other comments
>    don't obscure your overall opinion.
This is a much needed library in boost. I vote for it to be accepted.
> 
> Unordered containers are likely to become one of the most used Boost 
> libraries so I encourage you to review the library in order to see if 
> Unordered fulfills all your expectations.
I think this appears to be an excellent start. Performance will be an 
issue for me, but as I am in a position to do the needed comparisons 
myself (but don't have time yet) I can appreciate how more performance 
information could be very subjective to different peoples' needs.
Jamie
> 
> Ion Gaztañaga
> - Review Manager -
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
> 

Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk