Since the ImageConsumer accepts Hashtable<?,?> my thought is that the the GifDecoder should declare the field based on what it actually uses. If the other decoders only use String keys and String values, then I would suggest we change the field declaration to match.
Yes, it sounds reasonable. On 10/25/06, Nathan Beyer <[EMAIL PROTECTED]> wrote:
On 10/24/06, Oleg Khaschansky <[EMAIL PROTECTED]> wrote: > Nathan, could you, please tell why you changed the field properties in > these classes to > Hashtable<Object,Object> in two of them and to > Hashtable<String,String> in one of them (GifDecoder)? > > Look at the declaration in the ImageConsumer class: > void setProperties(Hashtable<?,?> props) > > It'd be better to have Hashtable<?,?> as a type in all 3 classes and > Hashtable<Object,Object> as an initial value for this field. A field type of Hashtable<?,?> would be painful, as you can't perform any sets when a type variable is ?. You'd have to cast it to Hashtable<Object,Object> before performing any sets. Since the ImageConsumer accepts Hashtable<?,?> my thought is that the the GifDecoder should declare the field based on what it actually uses. If the other decoders only use String keys and String values, then I would suggest we change the field declaration to match. -Nathan > > On 10/24/06, Oleg Khaschansky <[EMAIL PROTECTED]> wrote: > > > > Where are the tests for these decoders? How did you determine that > > > > they no longer worked? > > > > Unfortunately, these classes are not covered with the unit tests. > > > > I was running a simple test application that did something like this: > > Toolkit.getDefaultToolkit().getImage("image.jpg"); > > and if failed with a NPE. > > > > > I removed the final modifiers > > At the first glance it seems like the problem doesn't appear any more. > > > > > There were only 3-4 other fields that were finalized. Your email > > > mentioned "a lot of invalid modifications"; what are the other issues, > > > specifically? > > Only final fields. No other issues. But 3-4 in 3 classes - it is alot for me :) > > > > On 10/24/06, Nathan Beyer <[EMAIL PROTECTED]> wrote: > > > I removed the final modifiers; this only affected PngDecoder, > > > GifDecoder and JpegDecoder. I missed the comments in the fields of > > > JpegDecoder, that was my mistake. > > > > > > There were only 3-4 other fields that were finalized. Your email > > > mentioned "a lot of invalid modifications"; what are the other issues, > > > specifically? > > > > > > -Nathan > > > > > > On 10/24/06, Nathan Beyer <[EMAIL PROTECTED]> wrote: > > > > Where are the tests for these decoders? How did you determine that > > > > they no longer worked? > > > > > > > > I'll remove the final modifiers. > > > > > > > > -Nathan > > > > > > > > On 10/24/06, Oleg Khaschansky <[EMAIL PROTECTED]> wrote: > > > > > Hi, > > > > > > > > > > Rev. 465514 introduced a lot of invalid modifications to the > > > > > GifDecoder, PngDecoder and JpegDecoder. There were a number of fields > > > > > modified or initialized from the native code only, but they were > > > > > redeclared as final, so the decoders doesn't work properly any more. > > > > > > > > > > This revision has the following comment: > > > > > > > > > > Cleanup code > > > > > * Add if/else braces > > > > > * Add missing annotations > > > > > * Add type variables > > > > > * Use foreach loops > > > > > * etc > > > > > > > > > > I'd suggest to roll back this revision and redo the cleanup in the > > > > > more accurate way. > > > > > > > > > > Thanks, > > > > > Oleg > > > > > > > > > > > > > > >