[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