[OpenJDK 2D-Dev] Request for review: 7196571, 7196572, 7196573: javac warnings cleanup from Adopt OpenJDK bugathon

[Stuart Marks]

On 9/11/12 1:41 PM, Martijn Verburg wrote:
>> * By contrast lines 1035 and 1115 here are now much, much > 80 chars
>> http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.imageio/src/share/classes/javax/imageio/ImageIO.java.sdiff.html
>>
>> Also for both of these cases, you added .asSubClass(..).
>> Now when I look at the new code there, it looks a lot busier, all to avoid
>> one
>> simple (ImageWriterSpi) cast. Is it really worth it ?
>> And if the change is made, maybe you should check for ClassCastException.
>> I guess we already vulnerable to that, although its rather unlikely to occur
>> ...
>
> I'm more than happy to follow your lead on this one. I'll revert those
> two, we can
> always visit it again later with fresh eyes (I'm not that convinced that
> using asSubClass(..) is the way to go here either).

Using Class.forName(...).asSubClass(...) is a reasonable type-safe way of 
getting a Class<T> given a string name. However, it does change the location of 
any exception that might get thrown, and there might be a subtle interaction 
with the IIORegistry, so it might indeed be best to leave these untouched for 
the moment.

I looked through the webrevs and I noticed a couple minor things...

* 
http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-print/src/share/classes/sun/print/RasterPrinterJob.java.sdiff.html

  - lines 508-509 can be joined
  - lines 1311-1312 should probably be broken after the = operator,
    for consistency with other cases here
  - lines 1320-1321 can be joined
  - line 1910 can remove redundant parentheses

>> Don't forget you need a second reviewer for all the changes.
>
> OK, thanks - if another reviewer is following this thread then any
> assistance would be
> most welcome :-)

I'm not sure I count as a second reviewer. I did look quickly through all the 
changes, though, and the only issues I noticed were the ones I mentioned above.

s'marks