Review for CR 6728865 : Improved heuristics for Collections.disjoint() [updated]

Mike Duigou mike.duigou at oracle.com
Mon Jan 3 22:15:34 UTC 2011


On Dec 23 2010, at 13:24 , Ulf Zibis wrote:

> Am 23.12.2010 20:46, schrieb Mike Duigou:
>> I have updated the webrev with the javadoc changes (identical to below) and Ulf's suggested code changes:
>> 
>> http://cr.openjdk.java.net/~mduigou/6728865.3/webrev/
> 
> Thanks, I could convince you mostly.
> But, IMO, the inner comments are too expatiated ("less than O(N/2)" is used 3 times).

I've tried to favour clarity and simplicity of language over brevity for comments and mostly intended the comments for casual readers trying to understanding the operation of the method rather than future maintainers. This applies to the source code as well-- I've somewhat ignored "Optimizations" which make the source code smaller but no faster in execution. 

> Aren't the explanation comments from my last example clear enough and more fluently readable?
> You use "ceiling(n/2)". Didn't you mean "ceiling O(N/2)" for consistence?

The example refers to the total number of actual comparisons not it's O complexity so "ceiling(N/2)" is correct.

> "//than mere Collection's,iterate on c2."
> 2 spaces are missing
> 

One space added.

> "// One (or both) collections is/are empty. Nothing will match."
> Shorter:
> "// At least one collection is empty. Nothing will match."

Done.

> Javadoc:
> - the term '@throws NullPointerException' is duplicated, don't know if that would work ?

This is intentionally copying the style of Collection methods where two separate @throws NullPointerException clauses are also used.

> - there is a habit, maybe a rule, to insert 2 spaces after '.'

Only for those still using typewriters. :-) For javadoc comments the double spaces are collapsed to a single space in the HTML view and just take up precious characters in the 80 column line wrap. To my knowledge the 2 spaces after punctuation style has been abandoned.

Mike


More information about the core-libs-dev mailing list