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

[Andrew Azores]

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