[rfc][icedtea-web] Code cleanup patches

[Andrew Azores]

On 02/05/2014 12:24 PM, Andrew Azores wrote:
> On 02/04/2014 03:28 PM, Andrew Azores wrote:
>> On 02/04/2014 01:24 AM, Jacob Wisor wrote:
>>> On 02/03/2014 09:32 PM, Andrew Azores wrote:
>>>> Hi,
>>>>
>>>> The larger two of the three attached patches do a few simple things:
>>>> 1) make variables/parameters/fields final, where applicable. I feel 
>>>> this makes
>>>> it more clear which variables are used eg as accumulators or will 
>>>> be returned by
>>>> the method, etc. This also allows the removal of some local 
>>>> variables which were
>>>> created simply to have "final versions" of other variables, in 
>>>> order for them to
>>>> be captured by anonymous inner classes for example.
>>>> 2) prefix fields with "this" in very long methods. In shorter 
>>>> methods it can
>>>> remain clear that fields are fields since no declaration for that 
>>>> name can be
>>>> seen, but in very long methods, it can become more confusing.
>>>> 3) fix formatting. Eg remove useless comments, remove whitespace 
>>>> from "empty"
>>>> lines, remove trailing whitespace
>>>
>>> Looks good to me.
>>>
>>> Just on nit:
>>>
>>> diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java 
>>> b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>> [...]
>>> -            tracker.addResource(jar.getLocation(),
>>> -                    jar.getVersion(), file.getDownloadOptions(),
>>> +           this. tracker.addResource(jar.getLocation(),
>>> +                    jar.getVersion(), this.file.getDownloadOptions(),
>>>
>>> Please remove the space between this. and tracker.
>>
>> Ah, nice catch :)
>>
>>>
>>> You may also want to check for "){" malformed block openings and 
>>> replace them with ") {".
>>
>> Yea, might as well. And we should probably get stricter on this kind 
>> of thing during review, too.
>>
>>>
>>> Most of the exception parameters of catch clauses do not need to be 
>>> explicitly marked final because they are never assigned to hence 
>>> they become /effectively/ /final/. The OpenJDK compiler does take 
>>> this into account when generating byte code indeed. But, as I 
>>> understand the purpose of these patches was to increase readability.
>>>
>>> Jacob
>>
>> Pretty much just for readability's sake, yes.
>>
>> Thanks,
>>
>
> Huh, apparently my IDE was having some difficulties yesterday. Some of 
> the places where I prefixed a variable identifier with "this." were 
> actually within inner classes and so are actually invalid, but my 
> syntax highlighting was apparently nonfunctional and I wasn't quite 
> paying close enough attention evidently. This occurred three times in 
> JNLPClassLoader. If you apply these patches and find that you can't 
> build, well, this is why ;) I won't bother attaching the fixed patches 
> because they would be identical other than this.
>
> Thanks,
>

Holy... wow, apparently my IDE *really* had some problems going on 
yesterday, I went back and looked at the 1.4 backport and it's pretty 
mangled too. Please ignore the previous patches. *Actually* buildable 
ones are attached. :(

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: classloader_cleanup.patch
Type: text/x-patch
Size: 83698 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140205/94aa89e8/classloader_cleanup-0001.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: classloader_cleanup-1.4.patch
Type: text/x-patch
Size: 84605 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140205/94aa89e8/classloader_cleanup-1.4-0001.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: extensionjnlpsinapplet_makefile_ls.patch
Type: text/x-patch
Size: 462 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140205/94aa89e8/extensionjnlpsinapplet_makefile_ls-0001.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: securitydialogmessagehandler_cleanup.patch
Type: text/x-patch
Size: 8375 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140205/94aa89e8/securitydialogmessagehandler_cleanup-0001.patch 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: securitydialogmessagehandler_cleanup-1.4.patch
Type: text/x-patch
Size: 7555 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140205/94aa89e8/securitydialogmessagehandler_cleanup-1.4-0001.patch