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

Ulf Zibis Ulf.Zibis at gmx.de
Tue Jan 4 01:02:27 UTC 2011


Am 03.01.2011 23:15, schrieb Mike Duigou:
> 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.
OK, we are different. For me, redundancy usually reduces clarity. I would need more time to read and 
understand the comments, especially as a non-english native.

>   This applies to the source code as well-- I've somewhat ignored "Optimizations" which make the source code smaller but no faster in execution.
Well, I think a single swap (single CPU instruction) is little faster, than copying to local 
variables, and in Hotspot optimization case reduces register pressure. Anyway, your code reorders 
the arguments in more cases than necessary.
That's why I have constructed the decision maps to clearly see the common paths.

>
> The example refers to the total number of actual comparisons not it's O complexity so "ceiling(N/2)" is correct.
I was in error, as I finally noted (overseen ?).
But I still think, (N+1)/2 is more correct than ceiling(N/2).
N=2: match can be in 1st or 2nd position, so 1.5 comparisions in avarage!
N=3: match can be in 1st, 2nd or 3rd position, so 2 comparisions in avarage!
N=4: match can be in 1st to 4th position, so 2.5 comparisions in avarage!

>> "//than mere Collection's,iterate on c2."
>> 2 spaces are missing
> One space added.
The 2nd one is after the comma. ;-)

>
>> 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.
Hm, where exactly did yo find this style?


> 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.
IMO it still enhances the readability in the source code.

-Ulf




More information about the core-libs-dev mailing list