Hi all,

Please see the updated webrev http://cr.openjdk.java.net/~ntv/8143413/webrev.04/

Changes : Included the changes suggested by Stephen

Thanks and Regards,
Nadeesh

On 12/22/2015 12:30 AM, Stephen Colebourne wrote:
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


--
Thanks and Regards,
Nadeesh TV

Reply via email to