[icedtea-web] RFC: use Arrays.asList instead of custom implementation

Andrew Azores aazores at redhat.com
Mon Sep 16 08:53:35 PDT 2013


On 09/16/2013 11:39 AM, Omair Majid wrote:
> Hi Andrew,
>
> On 09/16/2013 09:27 AM, Andrew Azores wrote:
>> On 09/13/2013 05:19 PM, Omair Majid wrote:
>>> There's a couple of places in icedtea-web where we are using a custom
>>> method to do what's already being done by Arrays.asList. Lets remove
>>> this 'duplicate' code.
>> Looks good to me. Not sure how I feel about the "import static" for the
>> UnsignedAppletTrustConfirmationTest, but no big deal.
> Do you see a (potential) issue? Or do you (only) have a stylistic
> concern? There was already a static import, so I added another.
>
> Fixed in updated patch.
>
> Thanks,
> Omair
>

Just a stylistic concern :) I don't mind static imports for things like 
Assert.assertTrue, because it's already a bit of a redundant looking 
call. For things like Arrays.asList or Collections.synchronizedList I 
think keeping the name of the class the static method comes from just 
makes it a bit clearer to see what's happening, especially when the 
definition of the array and the method call are far enough apart that it 
might not be immediately obvious that the array is in fact an array.

Also one minor point I just thought of. Arrays.asList() returns an 
object that implements the List interface, but the List it returns is 
actually defined as being fixed-size, ie the "add/remove" optional 
operations for Lists are not actually implemented (beyond throwing an 
UnsupportedOperationException at least). The methods you've replaced 
return an ArrayList in particular, which does implement "add/remove." It 
doesn't look like these operations are needed anywhere affected by this 
change but it's something to double check I suppose.

Thanks,

-- 
Andrew A




More information about the distro-pkg-dev mailing list