On 12/06/2011 01:28 PM, Chris Hegarty wrote:
On 12/ 6/11 01:12 AM, David Holmes wrote:
Chris, Doug,
A few nits see below.
Cheers,
David
-----
As a matter of style can we ensure annotations are on separate lines. I
find this:
@SuppressWarnings("unchecked") E x = (E) items[takeIndex];
hard to read. (I hate seeing local variable annotations in the first
place - way too much clutter in my opinion.)
local variable annotations/declaration now on new line.
Is the reason for constructs like this:
HashEntry<K,V>[] tab = (HashEntry<K,V>[])new HashEntry<?,?>[cap];
that we can't utilize diamond? Otherwise it would nicely reduce to:
HashEntry<K,V>[] tab = new HashEntry<>[cap];
I see follow up between you, Remi, and Maruizio, on this. Also note
that we need to be able to compile with 1.6, so no diamond anyway. I
assume this code is good as stands?
In ConcurrentSkipListMap:
- doesn't the class-level unchecked annotation make the local-variable
annotation redundant in clone()
Right, but it is not wrong and may be useful if there is ever
refactoring of this code and the class level unchecked annotation is
removed. clone always needs to suppress unchecked. I'm ok either way.
Not true.
You only need a @SuppressWarnings if you use a return type which is
different from Object
and doesn't inherits from a class that already have a clone that doesn't
return an Object.
So here you need a @SuppressWarnings but it's not always the case.
By the way, there is no need to initialize the local variable clone to null.
[...]
Thanks,
-Chris.
cheers,
Rémi
