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

Omair Majid omajid at redhat.com
Mon Sep 16 09:40:12 PDT 2013


On 09/16/2013 11:53 AM, Andrew Azores wrote:
> 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.
>>
> 
> 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.

Sure, but this sort of problem only happens if you are using a
Java-unaware editor. Time to get a better editor? :)

> 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.

I ran the unit tests and they still pass. Also, it would be pretty bad
design if an input argument was modified unexpectedly.

I have pushed the patch:
http://icedtea.classpath.org/hg/icedtea-web/rev/f544f5b40bb7

Cheers,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681



More information about the distro-pkg-dev mailing list