[rfc][icetea-web] Resource Setter/Getter Refactoring

[Andrew Azores]

On 05/20/2014 02:35 PM, Lukasz Dracz wrote:
>
> ----- Original Message -----
> From: "Jiri Vanek" <jvanek at redhat.com>
> To: "Andrew Azores" <aazores at redhat.com>, "Lukasz Dracz" <ldracz at redhat.com>
> Cc: distro-pkg-dev at openjdk.java.net
> Sent: Tuesday, May 20, 2014 11:20:56 AM
> Subject: Re: [rfc][icetea-web] Resource Setter/Getter Refactoring
>
>
>> Otherwise I think this is good to go. Please send a new version with fixed formatting and testSetSize.
>>
>> Thanks,
>>
>> And new changelog!
>>
>> Thanx for clean up! Good start!-)
>>
>> J.
> Hello,
>
> I changed the '10' to a constant in testIncrementTransferred(), changed my eclipse format settings to use 4 spaces for indentation and added the entry to the Changelog as suggested by Jiri.
>
> Thanks everyone for the help,
> Lukasz D
>

Hi,

+    public void setDownloadVersion(Version dVersion) {
+    	downloadVersion = dVersion;
+    }


This kind of thing was fixed for setTransferred, so I'd like to see it 
fixed here as well, and for setSize, and any others I may have missed.

The comment on getTracker is also kind of questionable. I don't know if 
the @return is adding anything helpful here, and in any case, it's 
worded a bit awkwardly.

Finally, where you have 
"resource.incrementTransferred(downloadLocationFile.length())", are you 
sure that this replacement is equivalent to the original line, which 
directly set the "transferred" field to this value rather than 
incrementing it? ie are you sure that at this point, "transferred" will 
always be 0? Perhaps a comment here stating this would be useful, so 
that it's clear that this is really a "set transferred" even though it 
appears to be an increment.

Thanks,

-- 
Andrew A