RFR:8146218: Producing streams in java.time?

[Roger Riggs]

Hi,

Looks good.

I would want to verify that it is defined such that if it later needs to be
abstracted up to ChronoLocalDate and apply to compatible Chronologies
and respective local dates such as JapaneseDate there is no conflict.
It should not be an issue since Period implements ChronoPeriod.
In the respective implementations, the estimation/computation of the 
number of steps
would need to depend on the Chronology's definition of months.

I can sponsor this enhancement.

Thanks, Roger



On 1/20/16 10:15 AM, Stephen Colebourne wrote:
> I'm happy with the logic and specification of this proposal. I think it
> will be a useful addition.
>
> I'll let the Oracle team chime in to do a further review.
>
> thanks
> Stephen
>
>
>
> On 16 January 2016 at 13:31, Tagir F. Valeev <amaembo at gmail.com> wrote:
>
>> Hello!
>>
>> Thanks for review! Here's the updated patch:
>> http://cr.openjdk.java.net/~tvaleev/webrev/8146218/r2/
>>
>> SC> The docs say that if the end date is before the start date, the
>> SC> stream is empty. While I can see just about why LongStream.range()
>> SC> works that way, I don't think this API should. The minimum is an
>> SC> exception, but it would be easy to support negative in the
>> SC> days-only case or the months-only case. The problem is where there
>> SC> are both months/years and days and those should be exceptions.
>>
>> Now datesUntil(endExclusive) throws an IllegalArgumentException if end
>> date is before start date.
>>
>> datesUntil(endExclusive, step) supports negative periods. It throws
>> IllegalArgumentException if:
>> - step is zero
>> - step.toTotalMonths() and step.getDays() both non-zero and have
>> opposite sign
>> - step is negative and end date is after start date
>> - step is positive and end date is before start date
>>
>> Otherwise it works correctly: you can use
>> LocalDate.of(2016, 1, 1)
>>    .datesUntil(LocalDate.of(2015, 1, 1), Period.ofMonths(-1));
>>
>> Steps like Period.of(-1, -1, -1) are also supported.
>>
>> SC> The single-arg implementation uses plusDays() with an
>> SC> incrementing number. When the performance patch goes in, the
>> SC> proposed streaming implementation will be optimal for small
>> SC> streams but less optimal for large ones. The fastest way to loop
>> SC> over a list of dates would be to manually generate them by
>> SC> incrementing the day until it exceeds the length of month, and so
>> SC> on. However, this would be serial.
>>
>> As I understand, plusDays performance patch is already pushed.
>>
>> It's possible to write custom Spliterator which would use
>> previous.plusDays(1) in tryAdvance() and jump (via
>> LocalDate.of(startEpochDay+n)) in trySplit() if parallel processing is
>> requested. However this adds at least one additional class and more
>> complexity. I guess, such optimization can be considered later as
>> separate issue when API is stabilized.
>>
>> SC> As such, I think the best way to write this, taking account of
>> SC> how plusDays() is implemented, is as follows:
>>
>> SC> LongStream.range(start.toEpochDay(),
>> SC> end.toEpochDay()).mapToObj(LocalDate::ofEpochDay);
>>
>> Ok, now it's done this way.
>>
>> SC> In the period-based method, it would be best to capture the case
>> SC> where totalMonths == 0 and days > 0 and forward to another method
>> SC> that ignores months. That private method can share implementation
>> SC> with the public single-arg method (passing in 1).
>>
>> This case still more complex than one-day case as it requires division
>> and multiplication. Thus I'd keep these case separately. However I
>> simplified "months = 0" path using ofEpochDay, now it should be
>> faster.
>>
>> SC> The docs for the period-based method should specify the strategy
>> SC> that produces the results (in abstract terms). This needs to cover
>> SC> that the result is equivalent to always adding to the start period
>> SC> a multiple of the period.
>>
>> I added some clarifications, please check.
>>
>> SC> Some nits:
>> SC> I prefer to avoid @link in the first sentence. Just using 'stream'
>> would be sufficient.
>>
>> Done.
>>
>> SC> The first sentence should be a summary. In this case it probably has a
>> bit too much detail.
>>
>> Done.
>>
>> SC> The @return has 'values' on a new line when it could be on the same
>> line.
>>
>> I set now line length = 100 characters in my IDE. Is it ok?
>>
>> SC> If statements need braces.
>>
>> Done.
>>
>> With best regards,
>> Tagir Valeev.
>>
>>