RFR 7118066: Warnings in java.util.concurrent package (original) (raw)
David Holmes david.holmes at oracle.com
Tue Dec 6 01:12:23 UTC 2011
- Previous message: RFR 7118066: Warnings in java.util.concurrent package
- Next message: RFR 7118066: Warnings in java.util.concurrent package
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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.cast(item);
to
LinkedTransferQueue.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.
- Previous message: RFR 7118066: Warnings in java.util.concurrent package
- Next message: RFR 7118066: Warnings in java.util.concurrent package
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]