Hi all, Apologies for the long response, as with many of you JavaOne prep seems to get in the way of doing interesting coding work!
On 12 September 2012 20:03, Stuart Marks <[email protected]> wrote: > On 9/11/12 1:41 PM, Martijn Verburg wrote: >>> >>> 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. OK, left untouched in that case. > 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 I've fixed these and sent Artem an updated patch for his webrev >>> 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. Hopefully that's all that's required then :-) Cheers, Martijn
