RFR:8146218: Producing streams in java.time?

[Xueming Shen]

The proposed change looks fine with me.

sherman

On 1/23/16 1:40 PM, Roger Riggs wrote:
> 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.
>>>
>>>
>