RFR:8146218: Producing streams in java.time?

Tagir F. Valeev amaembo at gmail.com
Thu Jan 21 16:07:55 UTC 2016


Thank you for review! I did very small change in unit test comments to
make them less confusing. Here's updated webrev:

With best regards,
Tagir Valeev.

SC> I'm happy with the logic and specification of this proposal. I think it
SC> will be a useful addition.

SC> I'll let the Oracle team chime in to do a further review.

SC> thanks
SC> Stephen

SC> 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.

More information about the core-libs-dev mailing list