Codereview Request: 8011647: Add java.time.Instant methods to java.nio.file.attribute.FileTime

[Alan Bateman]

On 11/04/2013 21:47, Xueming Shen wrote:
> Hi
>
> As part of the JSR310 Date/Time project, following methods are proposed
> to be added into java.nio.file.attribute.FileTime to interoperate with 
> the
> new JSR310 time class Instant.
>
> public static FileTime from(Instant instant);
> public Instant toInstant();
>
> http://cr.openjdk.java.net/~sherman/8011647/webrev
Sherman and I chatted about adding from(Instant)/toInstant() a few times 
over the last week so most of my comments are already included.

Overall I think it has worked out well, just unfortunate that there is a 
bit of extra complexity because FileTime supports a wider time-line and 
we also went with the XML schema deviation from (older?) ISO 8601 for 
year 0000. In practical terms these aren't of course interesting for 
file times.

In terms of API then from(Instant)/toInstant() are simple and I don't 
think we have an urgent need for FileTimes to be directly formatted.

On toString then FileTime doesn't require that trailing zeros be 
stripped from the fraction of a second. If it is better to use 3-digit 
units that it is okay. Also if this is something that we got wrong in 
FileTime then we could change the spec without any compatibility impact.

Values outside of MIN/MAX aren't too interesting so hashCode using 
Instant hashCode should be fine. I had to read compareTo a few times to 
convince myself that it correctly orders FileTimes where one is created 
from an Instant and the other from a value outside of MIN/MAX. It might 
be useful to expand the comment to explain this further.

Thanks for expanding the existing test.  A minor point at line 117 is 
that you don't need "Random r" as there is rand already. The 
pre-existing tests loop over TimeUnit.values() rather than 
EnumSet.allOf(TimeUnit.class). In eqTime then it prints to System.out 
where the rest of the test prints to System.err before throwing the 
exception.

That's all I have.

-Alan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20130412/74017318/attachment.html