[icedtea-web] RFC: use Arrays.asList instead of custom implementation
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:
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