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

[Xueming Shen]

On 04/12/2013 05:55 AM, Alan Bateman wrote:
> 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.

I'm not sure if the 3-digit unit fitting is "better" or not. But xml spec cited in the
toString() API specifies that the "trailing zeros are optional" in its 3.2.3.1 "lexical
representatin" section and "trailing zeros are prohibited..." in 3.2.3.2 "canonical
representation" section. So I think trimming trailing zeros in FileTime.toString()
is just fine.

http://www.w3.org/TR/xmlschema-2/#deviantformats

>
> 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.

Added some wording.
>
> 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.
>
Updated.

http://cr.openjdk.java.net/~sherman/8011647/webrev/

Thanks!
-Sherman


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