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.)
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];
In ConcurrentSkipListMap:
- doesn't the class-level unchecked annotation make the local-variable
annotation redundant in clone()
- you added:
* @param s the stream
for readObject, but not for writeObject. Seems unnecessary for either.
- in the KeySet and Values classes why the changes from <E,Object> to
<E,?> ? Seems superfluous. Did it affect any warnings?
In Exchanger why suppress the serial warning rather than add the
serialVersionId? Elsewhere the id was added. (I prefer to suppress but
am querying consistency here)
In LinkedTransferQueue we changed:
this.<E>cast(item);
to
LinkedTransferQueue.<E>cast(item);
but in ArrayBlockingQueue we simply changed to: (E) item. Were these
cast methods introduced simply to localize the unchecked suppression?
Also in LinkedTransferQueue we went from:
E e;
while ( (e = poll()) != null) {
to
for (E e; (e = poll()) != null;) {
the side-effect on the loop condition is ugly in a for-loop. Why change
from the while? Why not change to a proper for-loop:
for (E e = poll(); e != null; e = poll()) {
In SynchronousQueue:
- the Transferer, TransferStack and TransferQueue classes have been
generified, but not the internal SNode and QNode classes. Generifying
nodes would take a bit of work, but I guess I don't see the point of
generifying the others.
- why stop using Collections.emptyIterator()?
- same comments on while-loop to for-loop conversions as for LTQ
On 6/12/2011 1:36 AM, Chris Hegarty wrote:
Cleanup warnings in the j.u.c. package.
This is a sync up with the warning fixes in Doug's CVS. There are also a
few style cleanups, import fixes, trivial local variable renaming,
typos, etc. But nothing too surprising!
http://cr.openjdk.java.net/~chegar/7118066/webrev.00/webrev/
-Chris.
P.S. I have already reviewed this, and the contribution is of course
from Doug.
