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

[Lukasz Dracz]

----- Original Message -----
From: "Andrew Azores" <aazores at redhat.com>
To: "Jie Kang" <jkang at redhat.com>, distro-pkg-dev at openjdk.java.net
Cc: "Lukasz Dracz" <ldracz at redhat.com>
Sent: Friday, May 16, 2014 5:28:39 PM
Subject: Re: [rfc][icetea-web] Resource Setter/Getter Refactoring

On 05/16/2014 04:23 PM, Jie Kang wrote:
> Hi,
>
> The patch privatizes the fields in the Resource class and includes appropriate setters/getters for the respective fields. Unit Tests are included.
>
> 2014-05-16  Lukasz Dracz  <ldracz at redhat.com>
>              Jie Kang      <jkang at redhat.com>
>
>      * netx/net/sourceforge/jnlp/cache/Resource.java:
>      (location, localFile, requestVersion, downloadVersion,
>      transferred, size, status) made fields private and added
>      setters and getters, and all calling sites refactored
>      * netx/net/sourceforge/jnlp/cache/ResourceTracker.java:
>      Calling sites refactored
>      * netx/net/sourceforge/jnlp/cache/ResourceUrlCreator.java:
>      Calling sites refactored
>      * tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTest.java:
>      (testGetLocation, testGetRequestVersion, testGetDownloadVersion,
>      testTransferredIsZero, testIncrementTransferred, testSizeIsNegativeOne,
>      testSetSize, testStatusIsCopied) added tests
>
>
> Thanks,
>
>Couple little things here.
>- Resource#getTracker() - javadoc comment needs to be cleaned up
>- Resource#getLocalFile() - same
>- nitpicky but I'd prefer the following convention for the setters:
>
>public void setLocalFile(File localFile) {
>     this.localFile = localFile;
>}
>
>rather than "localFile = lFile"
>
>- add a line of whitespace between each new method please
>- Resource#getStatusString() line was modified only by adding a trailing 
>space... clean this out of the patch
>- ResourceTracker#checkCache(), there are two semicolons after the 
>increment statement
>-- are you sure increment is the right operation here? It is probably 
>correct, but have you traced the execution to be sure that at this point 
>the field is always going to be 0? If not, you've changed the semantics 
>of this method. Perhaps, to be safe, you should expose a setter for the 
>transferred field.
>
>Thanks,
>
>-- 
>Andrew A

Hello, 

I took Andrew, Jiri and Omair's suggestions and revised our previous patch.

Thanks,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2014-05-20P-ResourceSetterGetters.patch
Type: text/x-patch
Size: 22587 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140520/decbaff2/2014-05-20P-ResourceSetterGetters-0001.patch>