The comment for the new LocalDate.EPOCH field should use 1970-01-01, not 1970-1-1. However, the code should omit the zeroes: Replace: LocalDate.of(1970, 01, 01) with LocalDate.of(1970, 1, 1)
The new method should follow the documentation of the similar method on ChronoLocalDateTime: * This combines this local date with the specified time and offset to calculate the * epoch-second value, which is the number of elapsed seconds from 1970-01-01T00:00:00Z. * Instants on the time-line after the epoch are positive, earlier are negative. The same applies to the new method on LocalTime: * This combines this local time with the specified date and offset to calculate the * epoch-second value, which is the number of elapsed seconds from 1970-01-01T00:00:00Z. * Instants on the time-line after the epoch are positive, earlier are negative. The same applies to the new method on OffsetTime: * This combines this offset time with the specified date to calculate the * epoch-second value, which is the number of elapsed seconds from 1970-01-01T00:00:00Z. * Instants on the time-line after the epoch are positive, earlier are negative. The test cases look good. thanks Stephen On 17 December 2015 at 18:13, nadeesh tv <nadeesh...@oracle.com> wrote: > > Hi all, > > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8143413/webrev.03/ > > Thanks and Regards, > Nadeesh > > On 12/4/2015 3:56 AM, Stephen Colebourne wrote: >> >> In the interests of harmony and getting something in, I'll accept that. >> >> I think LocalDate.EPOCH is probably better than LocalDate.EPOCHDAY >> >> Stephen >> >> >> >> On 3 December 2015 at 22:09, Roger Riggs<roger.ri...@oracle.com> wrote: >>> >>> Hi Sherman, >>> >>> It makes sense to me to provide the missing time/date as an explicit >>> parameter >>> for toEpochSeconds instead of assuming some constant. >>> >>> localDate.toEpochSeconds(LocalTime.MIDNIGHT, ZoneOffset.UTC); >>> localTime.toEpochSeconds(LocalDate.EPOCHDAY, ZoneOffset.UTC); >>> offsetTime.toEpochSeconds(LocalDate.EPOCHDAY); >>> >>> We'll have to add the LocalDate.EPOCHDAY constant. >>> >>> It makes it a bit heavier weight to use but can still be optimized and >>> won't >>> create garbage. >>> >>> Roger >>> >>> >>> >>> On 12/01/2015 12:33 PM, Xueming Shen wrote: >>>> >>>> On 12/1/15 6:36 AM, Stephen Colebourne wrote: >>>>> >>>>> As Roger says, these new methods are about performance as well as >>>>> conversion. >>>>> >>>>> While I fully acknowledge the time methods make an assumption, it is >>>>> not a crazy one, after all 1970-01-01 is just zero. >>>>> >>>>> Key I think is it allows: >>>>> long epochSecs = date.toEpochSeconds(offset) + >>>>> time.toEpochSeconds(offset); >>>>> to efficiently merge two objects without garbage. >>>> >>>> So it's not about j.t.LD/LT <-> j.u.Date, but instead of the clean >>>> approach >>>> >>>> LocalDate date = ... >>>> LocalDate time = ... >>>> ZoneOffset offset = ... >>>> >>>> ==> long spochSecs = LocalDateTime.of(date, >>>> time).toEpochSeconds(offset); >>>> >>>> we are adding APIs to provide a "fastpath" with the special assumption >>>> that the LocalDate "date" >>>> here is actually a "LocalDateTime" object ("date" + LocalTime.MIDNIGHT) >>>> and the LocalTime "time" >>>> again actually means a "LocalDateTime" (the "time" of 1970-01-01), to >>>> let >>>> the third party "libraries" >>>> to fool the basic date/time abstract in java.time package, simply to >>>> avoid creating the garbage >>>> middle man, localDateTime? it really does not sound right to me. >>>> >>>> First, if someone needs to mix/link LocalDate, LocalTime and Offset to >>>> epoch seconds in their >>>> libraries, they really SHOULD think hard about the conversion and make >>>> it >>>> right (it should not >>>> be encouraged to use these classes LocalDate, LocalTime and Offset >>>> without >>>> even understand >>>> what these classes are). But if we really have to provide such fastpath, >>>> personally I think it might >>>> be better either to provide these "utility/convenient" methods in a >>>> "utilities" class, or with an >>>> explicit date/time parameters (instead of the false assumption) for the >>>> missing date/time piece, >>>> such as >>>> >>>> localDate.toEpochSeconds(LocalTime.MIDNIGHT, offset); >>>> localTime.toEpochSeconds(LocalDate.EPOCHDAY, offset); >>>> >>>> Sherman >>>> >>>> >>>>> It also means that no-one has to think hard about the conversion, as >>>>> it is just there. It tends to be when people try to work this stuff >>>>> out for themselves that they get it wrong. >>>>> >>>>> Stephen >>>>> >>>>> >>>>> On 1 December 2015 at 14:21, Roger Riggs<roger.ri...@oracle.com> >>>>> wrote: >>>>>> >>>>>> Hi Sherman, >>>>>> >>>>>> On 11/30/2015 6:09 PM, Xueming Shen wrote: >>>>>>> >>>>>>> On 11/30/2015 01:26 PM, Stephen Colebourne wrote: >>>>>>>> >>>>>>>> Converting LocalDate<-> java.util.Date is the question with the >>>>>>>> largest number of votes on SO: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> http://stackoverflow.com/questions/21242110/convert-java-util-date-to-java-time-localdate/21242111 >>>>>>>> The proposed method is designed to make the conversion easier. It >>>>>>>> also >>>>>>>> plugs an obvious gap in the API. >>>>>>>> >>>>>>>> While the LocalTime/OffsetTime methods are of lower importance, not >>>>>>>> having them would result in inconsistency between the various >>>>>>>> classes. >>>>>>>> We've already added factory methods to LocalTime for Java 9, these >>>>>>>> are >>>>>>>> just the other half of the picture. >>>>>>>> >>>>>>> I'm not sure I understand the idea of "the proposed method is >>>>>>> designed >>>>>>> to >>>>>>> make the conversion easier", as the SO is mainly about >>>>>>> j.u.Date->LocalDate, >>>>>>> not the other way around, from LocalDate -> j.u.Date. >>>>>> >>>>>> I think the issue is about *other* libraries that manipulate time via >>>>>> epochSeconds >>>>>> not about j.u.Date conversions. The concern was about performance and >>>>>> creating garbage along the way. >>>>>> >>>>>> Roger >>>>>> >>>>>> >>>>>> >>>>>>> As I said in the previous email, it might be "common" to use the >>>>>>> j.u.Date >>>>>>> to >>>>>>> abstract a "local date" and/or a "local time" (no other choice) >>>>>>> before >>>>>>> java.time, >>>>>>> and now there is the need to provide a migration path from those >>>>>>> "local >>>>>>> date/ >>>>>>> time" to the j.t.LocalDate/Time. But convert backward from the new >>>>>>> date/time >>>>>>> type to the "old" j.u.Date should not be encouraged (guess this is >>>>>>> also >>>>>>> the >>>>>>> consensus we agreed on back to jsr203). >>>>>>> >>>>>>> What are the "factory methods" you are referring to here? >>>>>>> JDK-8133079, >>>>>>> The >>>>>>> LocalDate/LocalTime.ofInstant()? >>>>>>> (btw, these two methods see missing the "since 1.9/9" tag) >>>>>>> >>>>>>> It seems to me that the ofInstant(Instant, ZondId) is from a >>>>>>> "super-set" >>>>>>> of >>>>>>> date/time to a "sub-set", without any assumption of "default value", >>>>>>> it >>>>>>> is >>>>>>> similar to the conversion from zdt->ldt->ld/lt, and I can see the >>>>>>> "small" >>>>>>> improvement >>>>>>> >>>>>>> from| >>>>>>> Date input = new Date(); >>>>>>> LocalDatedate >>>>>>> =input.toInstant().atZone(ZoneId.systemDefault()).toLocalDate();| >>>>>>> >>>>>>> to >>>>>>> >>>>>>> |LocalDatedate >>>>>>> =LocalDate.ofInstant(input.toInstant(),ZoneId.systemDefault()));| >>>>>>> >>>>>>> The proposed pair toEpochSecond() however is doing the other way >>>>>>> around >>>>>>> and >>>>>>> with an unusual assumption of the missing date/time piece to a >>>>>>> "default >>>>>>> value" >>>>>>> (midnight, the epoch day). >>>>>>> >>>>>>> personally I think >>>>>>> >>>>>>> localDate.atTime(LocalTime.MIDNIGHT).toEpochSecond(ZoneOffset); >>>>>>> localTime.atDate(LocalDate.EPOCHDATE).toEpochSecond(ZoneOffset); >>>>>>> >>>>>>> is clean and good enough to help this backward conversion (with the >>>>>>> addition >>>>>>> of LocalDate.EPOCHDATE/DAY constant). Maybe, the vm can even help to >>>>>>> remove that LocalDateTime middle man, with some arrangement. >>>>>>> >>>>>>> -Sherman >>>>>>> >>>>>>>> Note that these methods are specifically not referencing >>>>>>>> java.util.Date itself. Epoch seconds is the appropriate intermediate >>>>>>>> form here, and still widely used. >>>>>>>> >>>>>>>> Stephen >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 30 November 2015 at 19:36, Xueming Shen<xueming.s...@oracle.com> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> On 11/30/2015 10:37 AM, Stephen Colebourne wrote: >>>>>>>>>> >>>>>>>>>> This is based on user difficulties picked up via Stack Overflow. >>>>>>>>>> These >>>>>>>>>> methods aim to provide a simpler and faster approach, particularly >>>>>>>>>> for >>>>>>>>>> cases converting to/from java.util.Date. >>>>>>>>> >>>>>>>>> Can you be a little more specific on this one? We now have >>>>>>>>> Instance<=> >>>>>>>>> Date, >>>>>>>>> and considerably we might add LocalDateTime<=> Date with an offset, >>>>>>>>> if >>>>>>>>> really >>>>>>>>> really desired (for performance? to save a Instant object as the >>>>>>>>> bridge?). >>>>>>>>> But I'm >>>>>>>>> a little confused about the connection among LocalDate/LocalTime, >>>>>>>>> epoch >>>>>>>>> seconds >>>>>>>>> and j.u.Date here. Are you saying someone wants to convert >>>>>>>>> >>>>>>>>> j.t.LocalDate -> epoch seconds -> j.u.Date >>>>>>>>> j.t.LocalTime -> epoch seconds -> j.u.Date >>>>>>>>> >>>>>>>>> and uses the converted j.u.Date to represent a local date (date >>>>>>>>> with >>>>>>>>> time >>>>>>>>> part to >>>>>>>>> be 0) and/or the local time (with year/month/day to be epoch time) >>>>>>>>> in >>>>>>>>> the >>>>>>>>> "old" >>>>>>>>> system which only has j.u.Date, and has to use the j.u.Date to >>>>>>>>> abstract >>>>>>>>> the >>>>>>>>> "local >>>>>>>>> date" and "local time"? >>>>>>>>> >>>>>>>>> I think we agreed back to JSR310 that we don't try to add such kind >>>>>>>>> of >>>>>>>>> "bridge/ >>>>>>>>> convenient/utility" methods into the new java.time package, but >>>>>>>>> only >>>>>>>>> in >>>>>>>>> the >>>>>>>>> old date/calendar classes, if really needed. So if these methods >>>>>>>>> are >>>>>>>>> only to >>>>>>>>> help >>>>>>>>> migrate/bridge between the "old" and "new" calendar systems, the >>>>>>>>> java.time >>>>>>>>> might not be the best place for them? >>>>>>>>> >>>>>>>>>> For the time cases, the convention of 1970-01-01 is natural and >>>>>>>>>> commonly used in many codebases. >>>>>>>>>> >>>>>>>>> I'm not sure about that, especially the "natural" part. It might be >>>>>>>>> "common" >>>>>>>>> in >>>>>>>>> the old days if you only have j.u.Date", and might be forced to use >>>>>>>>> 1970-01-01 >>>>>>>>> to fill in the "date" part even when you are really only interested >>>>>>>>> in >>>>>>>>> "time" part >>>>>>>>> of it in your app. One of the advantage of java.time.LDT/LD/LT is >>>>>>>>> now >>>>>>>>> we >>>>>>>>> have >>>>>>>>> separate abstract for these different need, I don't see the common >>>>>>>>> need >>>>>>>>> of >>>>>>>>> having a LocalTime only meas the "local time" of 1970-01-01, or I >>>>>>>>> misunderstood >>>>>>>>> something here. >>>>>>>>> >>>>>>>>> -Sherman >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> Stephen >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 30 November 2015 at 18:15, Xueming >>>>>>>>>> Shen<xueming.s...@oracle.com> >>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> While it is kinda understandable to have >>>>>>>>>>> LocalDate.toEpochSecond(...) >>>>>>>>>>> to get the epoch seconds since 1970.1.1, with the assumption of >>>>>>>>>>> the >>>>>>>>>>> time is at the midnight of that day. It is strange to have the >>>>>>>>>>> Local/OffsetTime >>>>>>>>>>> to have two public methods with the assumption of the "date" is >>>>>>>>>>> at >>>>>>>>>>> epoch >>>>>>>>>>> year/month/day. What's the use case/scenario for these two >>>>>>>>>>> proposed >>>>>>>>>>> methods? >>>>>>>>>>> >>>>>>>>>>> -Sherman >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 11/30/2015 06:36 AM, Stephen Colebourne wrote: >>>>>>>>>>>> >>>>>>>>>>>> The method Javadoc (specs) for each of the three new methods >>>>>>>>>>>> needs >>>>>>>>>>>> to >>>>>>>>>>>> be enhanced. >>>>>>>>>>>> >>>>>>>>>>>> For the date ones it needs to say >>>>>>>>>>>> "The resulting date will have a time component of midnight at >>>>>>>>>>>> the >>>>>>>>>>>> start of the day." >>>>>>>>>>>> >>>>>>>>>>>> For the time ones it needs to say >>>>>>>>>>>> "The resulting time will be on 1970-01-01." >>>>>>>>>>>> >>>>>>>>>>>> Some of the line wrapping in the tests looks like it is not >>>>>>>>>>>> indented >>>>>>>>>>>> correctly. >>>>>>>>>>>> >>>>>>>>>>>> Otherwise looks fine, >>>>>>>>>>>> thanks >>>>>>>>>>>> Stephen >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On 30 November 2015 at 11:50, nadeesh tv<nadeesh...@oracle.com> >>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> Hi all, >>>>>>>>>>>>> >>>>>>>>>>>>> Please review a fix for >>>>>>>>>>>>> >>>>>>>>>>>>> Bug Id -https://bugs.openjdk.java.net/browse/JDK-8143413 >>>>>>>>>>>>> >>>>>>>>>>>>> Issue - add toEpochSecond methods for efficient access >>>>>>>>>>>>> >>>>>>>>>>>>> webrev -http://cr.openjdk.java.net/~ntv/8143413/webrev.01/ >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> -- Thanks and Regards, >>>>>>>>>>>>> Nadeesh TV >>>>>>>>>> >>>>>>>>>> <div id="DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2"><table >>>>>>>>>> style="border-top: 1px solid #aaabb6; margin-top: 10px;"> >>>>>>>>>> <tr> >>>>>>>>>> <td style="width: 105px; padding-top: 15px;"> >>>>>>>>>> <a >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail" >>>>>>>>>> target="_blank"><img >>>>>>>>>> src="https://ipmcdn.avast.com/images/logo-avast-v1.png" >>>>>>>>>> style="width: >>>>>>>>>> 90px; height:33px;"/></a> >>>>>>>>>> </td> >>>>>>>>>> <td style="width: 470px; padding-top: 20px; >>>>>>>>>> color: >>>>>>>>>> #41424e; >>>>>>>>>> font-size: 13px; font-family: Arial, Helvetica, sans-serif; >>>>>>>>>> line-height: 18px;">This email has been sent from a virus-free >>>>>>>>>> computer protected by Avast.<br /><a >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail" >>>>>>>>>> target="_blank" style="color: #4453ea;">www.avast.com</a> >>>>>>>>>> </td> >>>>>>>>>> </tr> >>>>>>>>>> </table><a href="#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2" width="1" >>>>>>>>>> height="1"></a></div> > > > -- > Thanks and Regards, > Nadeesh TV >