Hi,
I added a test to confirm the return type as Stream<LocalDate>.
If there are no more comments, will push it tomorrow.
Webrev: http://cr.openjdk.java.net/~rriggs/webrev-time-stream-8146218/
Roger
On 1/28/2016 12:00 AM, Roger Riggs wrote:
Hi Tagir,
After the discussion, I agree.
The only addition I would like to see in the tests to add or modify
a test so that the it explicitly requires the Stream<LocalDate> at
compile time.
None of the new tests check that the result type signatures are
as expected.
Thanks, Roger
On 1/27/16 11:28 PM, Tagir F. Valeev wrote:
Hello!
It should be noted that there's already a precedent in JDK where
method returning stream is subclassed and returns the stream of more
concrete objects. I'm speaking about ZipFile and JarFile:
public class ZipFile {
public Stream<? extends ZipEntry> stream() {...}
}
public class JarFile extends ZipFile {
@Override
public Stream<JarEntry> stream() {}
}
Such generic stream declaration adds some difficulties for ZipFile
users. For example, consider this question:
http://stackoverflow.com/q/31455188/4856258
So in general I would like to avoid Stream<? extends ChronoLocalDate>.
With best regards,
Tagir Valeev.
RR> Hi Stephen, Tagir,
RR> On 1/27/2016 10:30 AM, Stephen Colebourne wrote:
On 27 January 2016 at 15:13, Roger Riggs <roger.ri...@oracle.com>
wrote:
On 1/26/2016 8:57 AM, Stephen Colebourne wrote:
Thus, adding the ChronoLocalDate methods later will make two
additional
methods available on LocalDate (as they will not override).
Since all the calendars are built on the same 24hour days and
epochDays, the computations
result would be the same regardless of the Chronology.
The existing LocalDate.until, compareTo, isBefore, isAfter,
isEqual methods already use the
ChronoLocalDate argument type to avoid having double the signatures.
Modifying the type of the argument to be ChronoLocalDate would not
modify the semantics
and would make it possible to avoid extra methods in the future.
I recommend changing the argument to ChronoLocalDate be consistent
with the existing
until method to keep the option open for a possible addition to
ChronoLocalDate
The LocalDate::datesUntil(ChronoLocalDate) method internals would be
unaffected as it operates off toEpochDay(). Worth noting that an
abstraction on the ChronoLocalDate interface would have to return
Stream<? extends ChronoLocalDate>.
RR> Right, Interestingly, none of the tests explicitly depend on the
return
RR> type of Stream<LocalDate>
RR> and only use methods that are in ChronoLocalDate. (Based on a quick
RR> prototype).
RR> But its enough to suggest that there should be some additional
test or
RR> use of the compile time types.
RR> A Stream<ChronoLocalDate> would be inconvenient and counter
intuitive.
RR> That's enough of a reason for me to keep the current signatures.
RR> Thanks for the comments, Roger
A LocalDate::datesUntil(ChronoLocalDate, Period) method would however
contain a mixture of Chrono and ISO specific types. Given how the
internals of the method depend on access to Period specific concepts
abstracting to ChronoPeriod would not be pleasant (if possible) As
such, this signature seems unwise.
But that gives two types of signature - an abstracted one and a
specific one:
LocalDate::datesUntil(ChronoLocalDate)
LocalDate::datesUntil(LocalDate, Period)
Again, it isn't clear that is better.
Stephen
