Re: RFR(s): 8133977 add specification for serial form of immutable collections

2016-05-17 Thread Stephen Colebourne
CollSer should not be public, especially not just for serialization reasons. Stuart, I disagree with using an int for the flags, it should be a byte. If you need future expansion you can use 0xff to indicate it with the parser reacting accordingly. That is the strategy for JSR 310 Stephen On 17

Re: RFR:JDK-8155823: Add date-time patterns 'v' and 'vvvv'

2016-05-11 Thread Stephen Colebourne
This seems fine by me. Stephen On 10 May 2016 at 19:25, nadeesh tv wrote: > Hi, > > Stephen, Roger Thanks for the comments. > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8155823/webrev.04/ > > Apart from the fix, made a correction in the java doc of

Re: RFR:JDK-8155823: Add date-time patterns 'v' and 'vvvv'

2016-05-09 Thread Stephen Colebourne
This line: * v generic time-zone name zone-name Pacific Standard Time; PT should be * v generic time-zone name zone-name Pacific Time; PT Missing space before "false" at line 1226: appendInternal(new ZoneTextPrinterParser(textStyle,

Re: RFR(m): 8139233u1 add initial compact immutable collection implementations

2016-05-09 Thread Stephen Colebourne
On 9 May 2016 at 15:25, Remi Forax wrote: > CollSer implementation can be improved to avoid the ugly switch/case by > replacing the constant list by an enum, CollSer is modelled on JSR-310 code which uses an int. An enum would be a huge overkill for this use case. Stephen

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

2016-05-09 Thread Stephen Colebourne
. > > Thanks, > Bhanu > > -Original Message- > From: Stephen Colebourne [mailto:scolebou...@joda.org] > Sent: Monday, May 09, 2016 4:43 PM > To: core-libs-dev > Subject: Re: RFR 8066291: ZoneIdPrinterParser can be optimized > > One point - the Javadoc for th

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

2016-05-09 Thread Stephen Colebourne
. > > Thanks, > Bhanu > > -Original Message- > From: Stephen Colebourne [mailto:scolebou...@joda.org] > Sent: Friday, May 06, 2016 3:54 PM > To: core-libs-dev > Subject: Re: RFR 8066291: ZoneIdPrinterParser can be optimized > > The set of zones can only increase, i

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

2016-05-06 Thread Stephen Colebourne
a localized implementation with a simple model. > > Roger > > > On 5/5/2016 1:43 PM, Stephen Colebourne wrote: >> >> On reflection, both your and my solution have a race. >> >> the size method, is a clear check-then-act >> >> the read-only method u

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

2016-05-05 Thread Stephen Colebourne
esigning an API. > > Exposing a mutable Hashset was probably a mistake but one that cannot be > corrected > now. The optics aren't concerning to me. > > SharedSecrets are much messier and not appropriate. > > Adding a method to provide what was originally needed is a clea

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

2016-05-05 Thread Stephen Colebourne
I fail to see why adding a new read-only method alongside the existing method adds any more value to the API than adding a new size method. At least with the size method the API is still sensible - a mutable and immutable method alongside each other shouts out that a mistake was made. A size

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-05 Thread Stephen Colebourne
On 5 May 2016 at 02:30, Stuart Marks wrote: >> I disagree with altering the iteration order. Guava's ImmutableSet and >> ImmutableMap have reliable iteration specified to match creation >> order. This aspect of the design is very useful. > > > I think this is a reasonable

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

2016-05-05 Thread Stephen Colebourne
Fine by me thanks Stephen On 5 May 2016 at 10:57, Bhanu Gopularam <bhanu.prakash.gopula...@oracle.com> wrote: > Please see the updated webrev: > http://cr.openjdk.java.net/~bgopularam/bhanu/JDK-8066291/webrev.01/ > > Thanks, > Bhanu > > -Original Message-

Re: RFR 8066291: ZoneIdPrinterParser can be optimized

2016-05-05 Thread Stephen Colebourne
In ZoneRulesProvider.getAvailableZoneIdsSize() there is no need for the trailing paragraph tag in the Javadoc. Otherwise, fine by me. thanks Stephen On 5 May 2016 at 10:10, Bhanu Gopularam wrote: > Hi all, > > > > Please review fix following issue > > > >

Re: RFR(m): 8139233 add initial compact immutable collection implementations

2016-05-04 Thread Stephen Colebourne
A new instance is being created each time for size-zero lists/sets/maps. This should return a singleton. The serialization proxy is reminiscent of those in JSR-310 but less space efficient. Once per stream, the proxy will write "java.util.ImmutableCollections.SerialProxy", whereas the JSR-310 one

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

2016-05-04 Thread Stephen Colebourne
Fine by me. thanks Stephen On 4 May 2016 at 08:13, nadeesh tv <nadeesh...@oracle.com> wrote: > Hi, > > Updated the webrev http://cr.openjdk.java.net/~ntv/8148949/webrev.03/ > > Thanks and Regards, > Nadeesh > > On 5/3/2016 8:37 PM, Stephen Colebourne wrote: >>

Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

2016-05-03 Thread Stephen Colebourne
Letters "Q", "q", "M", "L", "d", "D", "F", "h", "H", "k", "K", "m", "s" and no doubt others use NORMAL via appendValue(field); Changing these to use NOT_NEGATIVE would be a big change and doesn't seem justified. For "D", "DD" and "DDD" these seem to be the best balance (as discussed

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

2016-05-03 Thread Stephen Colebourne
gt; > > > On 4/28/2016 4:04 PM, nadeesh tv wrote: > > Hi all, > Thanks Stephen for the comments. > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8148949/webrev.02/ > > Regards, > Nadeesh > > > On 4/28/2016 7:58 PM, Stephen Colebourne wrote: >

Re: RFR:JDK-8148949:DateTimeFormatter pattern letters 'A','n','N'

2016-04-28 Thread Stephen Colebourne
I'd like to see the test cases in test_secondsPattern() check the result of the parse (by passing more arguments from data_secondsPattern) Otherwise looks good. Stephen On 28 April 2016 at 14:12, nadeesh tv wrote: > Hi all, > Please see the updated webrev >

Re: RFR(m): 8140281 deprecate Optional.get()

2016-04-27 Thread Stephen Colebourne
of the > noise goes away. > > > On 4/27/2016 1:42 AM, Stuart Marks wrote: >> >> On 4/26/16 3:43 AM, Stephen Colebourne wrote: >>> >>> In OpenGamma Strata we have 546 uses of Optional.get(). Renaming this >>> would be painful and add no value. >>

Re: RFR(m): 8140281 deprecate Optional.get()

2016-04-27 Thread Stephen Colebourne
On 26 April 2016 at 22:55, Brian Goetz wrote: > As the person who chose the original (terrible) name, let me weigh in... We start from a different premise - I do not think that get() is a terrible name. Nor was it the biggest API mistake in Java 8 (I've come to believe

Re: RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

2016-04-26 Thread Stephen Colebourne
This change introduces inconsistencies and problems. For example, it will parse up to 19 digits for "D" but only up to 2 digits for "d". The following would be better: *D 1 appendValue(ChronoField.DAY_OF_YEAR) *DD 2 appendValue(ChronoField.DAY_OF_YEAR, 2, 3,

Re: RFR(m): 8140281 deprecate Optional.get()

2016-04-26 Thread Stephen Colebourne
In OpenGamma Strata we have 546 uses of Optional.get(). Renaming this would be painful and add no value. While I understand the pain from some developers not understanding the feature, this is hardly unique in the world of Java. Developers learn the right way of doing something soon enough. And

Re: RFR:JDK-8031085:DateTimeFormatter won't parse dates with custom format "yyyyMMddHHmmssSSS"

2016-04-19 Thread Stephen Colebourne
TV > > > > > On 4/18/2016 3:03 AM, Stephen Colebourne wrote: >> >> The updated spec at line 670 is not clear - the adjacent parsing mode >> only applies when in strict mode. Suggest a new sentence before the >> lenient mode one: "In strict mode, if the minimum

Re: RFR:JDK-8148947:DateTimeFormatter pattern letter 'g'

2016-04-17 Thread Stephen Colebourne
Looks fine to me Stephen On 30 March 2016 at 19:11, nadeesh tv wrote: > Hi all, > > BUG ID : https://bugs.openjdk.java.net/browse/JDK-8148947 > > Webrev : http://cr.openjdk.java.net/~ntv/8148947/webrev.00/ > > Added new pattern letter 'g' for Modified Julian day. > >

Re: RFR:JDK-8031085:DateTimeFormatter won't parse dates with custom format "yyyyMMddHHmmssSSS"

2016-04-17 Thread Stephen Colebourne
The updated spec at line 670 is not clear - the adjacent parsing mode only applies when in strict mode. Suggest a new sentence before the lenient mode one: "In strict mode, if the minimum and maximum widths are equal and there is no decimal point then the parser will participate in adjacent value

Re: RFR:JDK-8154050:java.time.format.DateTimeFormatter can't parse localized zone-offset

2016-04-17 Thread Stephen Colebourne
The LDML spec indicates that the "GMT" string should be localized: http://unicode.org/repos/cldr/trunk/specs/ldml/tr35-dates.html#Using_Time_Zone_Names The text of appendLocalizedOffset() is written with the intention of the output being localized (otherwise, what is the point of the method!) I

Re: RFR(m): 8145468 deprecations for java.lang

2016-04-15 Thread Stephen Colebourne
FWIW. I would prefer to see these constructors removed and the JLS simplified at the earliest possible date. Not using these classes as the value types for primitives in Valhalla would be very confusing. Stephen On 15 April 2016 at 08:49, Remi Forax wrote: > Hi Heinz, > i

Re: RFR:JDK-8148849:Truncating Duration

2016-03-30 Thread Stephen Colebourne
it == ChronoUnit.SECONDS also > > > Regards, > Nadeesh TV > > > On 3/29/2016 6:10 PM, Stephen Colebourne wrote: >> >> We're almost there, but looking at the tests, it looks like the >> behaviour is wrong: >> >> The intended behaviour is that >&g

Re: RFR:JDK-8148849:Truncating Duration

2016-03-29 Thread Stephen Colebourne
We're almost there, but looking at the tests, it looks like the behaviour is wrong: The intended behaviour is that -20.5mins (minus 20 minutes 30 secs) should truncate to -20mins -2.1secs truncate to -2secs Note that the truncation is different to Instant here. An Instant truncates towards the

Re: RFR:JDK-8148950 :Enhance ChronoField Javadoc

2016-03-29 Thread Stephen Colebourne
+1 Stephen On 29 March 2016 at 13:15, nadeesh tv wrote: > Hi all, > > Please see the doc changes > http://cr.openjdk.java.net/~ntv/8148950/webrev.00/ > Bug ID https://bugs.openjdk.java.net/browse/JDK-8148950 > > -- > Thanks and Regards, > Nadeesh TV >

Re: PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries

2016-03-29 Thread Stephen Colebourne
On 28 March 2016 at 22:41, Xueming Shen wrote: > It's a tricky call. To be honest, as I said at the very beginning, I'm not > sure whether > or not it's a good idea and worth the efffort to push this into the j.u.zip > package to > support the "traditional PKWare

Re: Review request for JDK-8151065 : Typo in java.time.Instant until(Temporal endExclusive, TemporalUnit unit)

2016-03-21 Thread Stephen Colebourne
+1 Stephen On 21 March 2016 at 12:28, Abhijit Roy wrote: > Hi all, > > > > Please review a fix for Bug - > https://bugs.openjdk.java.net/browse/JDK-8151868 > > > > Bug - Typo in java.time.Instant until(Temporal endExclusive, TemporalUnit > unit) > > > > Webrev -

Re: RFR 9: 8085887 : java.time.format.FormatStyle.LONG or FULL causes unchecked exception

2016-03-14 Thread Stephen Colebourne
thanks Stephen On 14 March 2016 at 20:17, Roger Riggs <roger.ri...@oracle.com> wrote: > Hi Stephen, > > Thanks for the comments and suggestions. > > I have no issue cleaning it all up in one change. > > Updated webrev: > http://cr.openjdk.java.net/~rriggs/webrev-

Re: RFR 9: 8085887 : java.time.format.FormatStyle.LONG or FULL causes unchecked exception

2016-03-12 Thread Stephen Colebourne
And just to note that only ofLocalizedTime and ofLocalizedDateTime are affected by this text. The ofLocalizedDate() method doesn't seem to use the time-zone, so the Javadoc should be vaguer. On 12 March 2016 at 09:47, Stephen Colebourne <scolebou...@joda.org> wrote: > I think the new t

Re: RFR 9: 8085887 : java.time.format.FormatStyle.LONG or FULL causes unchecked exception

2016-03-12 Thread Stephen Colebourne
I think the new text needs should be more specific: The {@code FULL} and {@code LONG} styles typically require a time-zone. When formatting using these styles, a {@code ZoneId} must be available, either by using {@code ZonedDateTime} or {@link DateTimeFormatter#withZone}. (testing shows that the

Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time

2016-03-04 Thread Stephen Colebourne
> "to represent" -> remove as unnecessary in all places >>> >>> IsoChronology: >>> "to represent" -> remove as unnecessary in all places >>> >>> These should be fixed to cleanup the specification. >>> >>> The impl

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-03-04 Thread Stephen Colebourne
n, Roger Thanks for the comments. > > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8032051/webrev.04/ > > > Regards, > Nadeesh > > > > On 3/1/2016 12:29 AM, Stephen Colebourne wrote: >> >> I'm happy to go back to the spec I proposed

Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time

2016-03-02 Thread Stephen Colebourne
ev.02/ > > Regards, > Nadeesh TV > > > On 3/2/2016 5:41 PM, Stephen Colebourne wrote: >> >> Remove "Subclass can override the default implementation for a more >> efficient implementation." as it adds no value. >> >> In the default implement

Re: RFR:JDK-8030864:Add an efficient getDateTimeMillis method to java.time

2016-03-02 Thread Stephen Colebourne
Remove "Subclass can override the default implementation for a more efficient implementation." as it adds no value. In the default implementation of epochSecond(Era era, int yearofEra, int month, int dayOfMonth, int hour, int minute, int second, ZoneOffset zoneOffset) use prolepticYear(era,

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-29 Thread Stephen Colebourne
fter the hour > may be confusing. > > In the specification of appendOffset(pattern, noOffsetText) how about: > > "When parsing in lenient mode, the longest valid pattern that matches the > input is used. Only the hours are mandatory, minutes and seconds are > optional.&q

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
de used in the other fields. > Otherwise, the pattern is irrelevant except for whether it contains a ":" > and makes > the spec nearly useless. > > Roger > > > > On 2/26/2016 12:09 PM, Stephen Colebourne wrote: >> >> On 26 February 2016 at 15:00, Roger Riggs

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
gits not included in the requested pattern. > > Separately, this is specifying the new lenient behavior of > appendOffset(pattern, noffsetText). > In that case, I don't think it will be understood that patterns 'shorter' > than the input will > gobble up extra digits and ':'s. &g

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
gt; + {"+HH", "+0101", 3660}, > + {"+HH", "+010101", 3661}, > + {"+HH", "+01", 3600}, > + {"+HH", "+01:01", 3660}, > + {"+HH", "+01:01:01", 3661}, > > Thanks, Roger > &

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
ses! Stephen On 25 February 2016 at 15:44, nadeesh tv <nadeesh...@oracle.com> wrote: > > Hi all, > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8032051/webrev.02/ > > Thanks and Regards, > Nadeesh > > On 2/23/2016 5:17 PM, Stephen Colebourne

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-23 Thread Stephen Colebourne
Thanks for the changes. In `DateTimeFormatter`, the code should be .parseLenient() .appendOffsetId() .parseStrict() and the same in the other case. This ensures that existing callers who then embed the formatter in another formatter (like the ZONED_DATE_TIME constant) are unaffected. The

Re: RFR 9: 8142539 : Incorrect definition of ZoneOffset.MIN and other javadoc cleanup

2016-02-10 Thread Stephen Colebourne
The plusHours() change loses something in the fix. previously, it had an example for summer-to-summer, summer-to-winter, and winter-to-winter. Now the last of these has gone. This would be better: Adding one hour to 01:30+02:00 will result in 02:30+02:00 (both in summer time) Adding one hour to

Re: RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations

2016-02-03 Thread Stephen Colebourne
Yes, looks fine thanks Stephen On 1 February 2016 at 06:18, nadeesh tv wrote: > Hi all, > > Please review following > > Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747 > > Solution: Handled the negative duration separately > > webrev :

Re: RFR 9: 8143016 : java/time/test/java/time/TestClock_System.java failed intermittently

2016-02-01 Thread Stephen Colebourne
Fine by me Stephen On 1 February 2016 at 16:41, Roger Riggs wrote: > Please review a test improvement to address an intermittent test failure in > the java.time System Clock test for microsecond support. > > webrev: >

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

2016-01-29 Thread Stephen Colebourne
nit.SECONDS); >>>>>> >>>>>> Instead of: >>>>>>TimeUnit tu = TimeUnit.SECONDS; >>>>>> ChronoUnit cu = tu.toChronoUnit(); >>>>>> >>>>>> >>>>>> Minor edits please: >>>>>&g

Re: RFR:8146218: Producing streams in java.time?

2016-01-27 Thread Stephen Colebourne
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

Re: RFR: JDK-8147912: test "parseWithZoneWithoutOffset" of java/time/tck/java/time/format/TCKDTFParsedInstant.java fail on de_DE locale

2016-01-27 Thread Stephen Colebourne
>From the name of the test and looking at the code, running in English only is fine. Stephen On 27 January 2016 at 05:53, Masayoshi Okutsu wrote: > Looks OK to me. But I'd like some java.time people to review this change > to see if the intention of this test is to

Re: RFR:8146218: Producing streams in java.time?

2016-01-26 Thread Stephen Colebourne
onology'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 additi

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

2016-01-25 Thread Stephen Colebourne
Typo "TimeUnitequivalent" Otherwise looks good. thanks Stephen On 25 January 2016 at 15:25, nadeesh tv wrote: > Hi all, > > Please review a fix for conversion between Chronounit and Timeunit > > Bug ID : https://bugs.openjdk.java.net/browse/JDK-8141452 > > webrev:

Re: RFR:8146218: Producing streams in java.time?

2016-01-20 Thread Stephen Colebourne
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 wrote: > Hello! > > Thanks for review! Here's the

Map.Entry methods for streams

2016-01-15 Thread Stephen Colebourne
I've had a morning of discussion about streaming Map this morning. While there is clearly no appetite for a JDK MapStream right now, it does seem that two additional methods on Map.Entry could help. Two of the common cases when streaming over Map.Entry are to transform the keys and to transform

Re: Map.Entry methods for streams

2016-01-15 Thread Stephen Colebourne
So, this proposal is essentially trying to add something small to JDK 9 in advance of the long wait for value types and a full solution to the problem space. A stream of Map.Entry is painful today, and my idea or Remi's would definitely assist without pushing the API too far (I'm not such a fan of

Re: RFR: JDK-8068803:Performance of LocalDate.plusDays could be better

2016-01-09 Thread Stephen Colebourne
gt; > (With Stephen's update below). > > Roger > > > On 1/8/2016 6:56 AM, Stephen Colebourne wrote: > > As I mentioned in my email: > > Rather than doing: >return withDayOfMonth((int) dom); > or >return LocalDate.of(year, month, (int) dom); > you

Re: RFR: JDK-8068803:Performance of LocalDate.plusDays could be better

2016-01-09 Thread Stephen Colebourne
egards, > Nadeesh > > > On 1/9/2016 4:58 PM, Stephen Colebourne wrote: >> >> Thanks for the update. You should have a test case for the exception >> thrown by the checkValidValue() >> Stephen >> >> On 9 January 2016 at 09:33, nadeesh tv <nadeesh

Re: RFR: JDK-8068803:Performance of LocalDate.plusDays could be better

2016-01-08 Thread Stephen Colebourne
com> wrote: > Hi all, > > Thanks Stephen for the comments > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8068803/webrev.02/ > Regards, > Nadeesh > > On 1/7/2016 6:15 PM, Stephen Colebourne wrote: >> >> I updated the benchmark with this cha

Re: RFR:8146218: Producing streams in java.time?

2016-01-06 Thread Stephen Colebourne
Thanks for the patch. The docs say that if the end date is before the start date, the stream is empty. While I can see just about why LongStream.range() works that way, I don't think this API should. The minimum is an exception, but it would be easy to support negative in the days-only case or

Re: [PING] PoC for JDK-4347142: Need method to set Password protection to Zip entries

2016-01-06 Thread Stephen Colebourne
Just to note that I once needed this and even commented on Stack Overflow about it: http://stackoverflow.com/questions/166340/write-a-password-protected-zip-file-in-java I'd just note that setting the password on each entry is a bit painful, as you'd normally expect to just set the password once.

Re: Producing streams in java.time?

2015-12-23 Thread Stephen Colebourne
JSR-310 was developed in parallel with the Stream API. While we briefly considered adding some stream methods, it wasn't obvious what would be of value. A key question is whether the methods are either specific to a narrow use case or more general. Specific methods like months() on Year are very

Re: RFR:8143413:add toEpochSecond methods for efficient access

2015-12-22 Thread Stephen Colebourne
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)

Re: RFR:8143413:add toEpochSecond methods for efficient access

2015-12-21 Thread Stephen Colebourne
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/20

Re: Review request for JDK-8066982: ZonedDateTime.parse() returns wrong ZoneOffset around DST fall transition

2015-12-21 Thread Stephen Colebourne
but the data is stable and it seems > unavoidable in this case. > > - Are all of the data cases necessary to validate the specification? >Redundant cases extend the testing time without adding more confidence to > the quality. > > Thanks, Roger > > > On 12/10/201

Re: RFR:JDK-8145166 : Duration.toString violates specification

2015-12-21 Thread Stephen Colebourne
Looks good to me. thanks Stephen On 19 December 2015 at 13:45, nadeesh tv wrote: > Hi all, > > Please review > > Bug Id - https://bugs.openjdk.java.net/browse/JDK-8145166 > > Issue - Duration.toString violates specification for Durations which have > value in the range

Re: Review request for JDK-8066982: ZonedDateTime.parse() returns wrong ZoneOffset around DST fall transition

2015-12-14 Thread Stephen Colebourne
e/no function, the tests would be a bit cleaner > without it. > > - Typically, test that have data dependencies (such as the timezone > data) cannot be used for > compatibility to the specification, but the data is stable and it seems > unavoidable in this case. > > - A

Re: Review request for JDK-8066982: ZonedDateTime.parse() returns wrong ZoneOffset around DST fall transition

2015-12-14 Thread Stephen Colebourne
extend the testing time without adding more confidence to > the quality. > > Thanks, Roger > > > > On 12/10/2015 11:00 AM, Stephen Colebourne wrote: >> >> I believe this is suitable for committing, thanks, other reviews welcome! >> Stephen >> >> >

Re: RFR:JDK-8032510 : Add java.time.Duration.dividedBy(Duration)

2015-12-11 Thread Stephen Colebourne
Fine by me. Stephen On 11 December 2015 at 11:53, nadeesh tv <nadeesh...@oracle.com> wrote: > > Hi all, > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8032510/webrev.03/ > Regards, > Nadeesh TV > > > On 12/11/2015 4:45 PM, Stephen Colebourn

Re: RFR:JDK-8032510 : Add java.time.Duration.dividedBy(Duration)

2015-12-11 Thread Stephen Colebourne
Missing blank line after the new method. Typo: "diviosr" Replace: Objects.requireNonNull(divisor, "divisor is null"); with Objects.requireNonNull(divisor, "divisor"); to match existing JSR-310 code. Test case looks fine. thanks Stephen On 11 December 2015 at 11:07, nadeesh tv

Re: Review request for JDK-8066982: ZonedDateTime.parse() returns wrong ZoneOffset around DST fall transition

2015-12-10 Thread Stephen Colebourne
I have modified the fix and test cases as per inputs given by Stephen. Also, > I have added the javadocs changes in this patch which were proposed in the > bug. > > Bug link is: https://bugs.openjdk.java.net/browse/JDK-8066982 > > > Regards, > Ramanand. > >

Re: Review request for JDK-8066982: ZonedDateTime.parse() returns wrong ZoneOffset around DST fall transition

2015-12-09 Thread Stephen Colebourne
The logic looks fine. In the main code, this part .getLong(INSTANT_SECONDS); can be replaced with .toEpochSecond(); which will be slightly faster. In the test case, this part .plus(15, ChronoUnit.MINUTES); can be replaced with .plusMinutes(15) And .with(ChronoField.OFFSET_SECONDS,

Re: RFR(m): update 2: JEP 269 initial API and skeleton implementation (JDK-8139232)

2015-12-04 Thread Stephen Colebourne
Sorry for my delayed review. Basically, I'm happy with this. Some oddments: Map.ofEntries(Entry...) "The entry objects themselves are not stored in the map." This seems wrong. `Entry` might be implemented as a value, not an object in the future. And if so, it might form part of the state of an

Re: RFR:8143413:add toEpochSecond methods for efficient access

2015-12-03 Thread Stephen Colebourne
.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

Re: RFR:JDK-8144349: @since tag missed

2015-12-01 Thread Stephen Colebourne
Those are not the right methods on LocalDate and LocalTime Stephen On 1 December 2015 at 16:50, nadeesh tv wrote: > Hi all, > Please review a fix for >BugID - https://bugs.openjdk.java.net/browse/JDK-814434 >Issue - while fixing JDK-8071919, JDK-8133079 I

Re: RFR:8143413:add toEpochSecond methods for efficient access

2015-12-01 Thread Stephen Colebourne
.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 vote

Re: RFR: JDK-8142936:Additional java.time.Duration methods

2015-11-30 Thread Stephen Colebourne
etSeconds()? > > The toXXXPart tests could have used a single DataProvider with > the values supplied for each unit to reduce the duplication. > > Thanks, Roger > > > > > > On 11/30/2015 09:29 AM, Stephen Colebourne wrote: >> >> I think thats ready to

Re: RFR:8143413:add toEpochSecond methods for efficient access

2015-11-30 Thread Stephen Colebourne
ically 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

Re: RFR:8143413:add toEpochSecond methods for efficient access

2015-11-30 Thread Stephen Colebourne
e 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: >>

Re: RFR: JDK-8142936:Additional java.time.Duration methods

2015-11-27 Thread Stephen Colebourne
"Overall, looks pretty good. There a a number of double spaces that should be single spaces in the Javadoc. Change "This is based on the standard definition of a day has 24 hours." to "This is based on the standard definition of a day as 24 hours." ("has" to "as") There are three places to fix

Re: RFR:JDK-8071919 :Add java.time.Clock.tickMillis(ZoneId zone) method

2015-11-13 Thread Stephen Colebourne
Looks good to me thanks Stephen On 13 November 2015 at 11:00, nadeesh tv wrote: > Hi all, > > Please review a fix for Bug Id > -https://bugs.openjdk.java.net/browse/JDK-8071919 > > Issue - Add java.time.Clock.tickMillis(ZoneId zone) method > > webrev -

Re: RFR:JDK-8071919 :Add java.time.Clock.tickMillis(ZoneId zone) method

2015-11-13 Thread Stephen Colebourne
ks based on a different timezone". Something like > "toMillisPrecisionClock" sounds better to me (but I might have the > terminology wrong). > > Gruss > Bernd > > -- > http://bernd.eckenfels.net > > -Original Message- > From: Stephen Colebourne <

Re: RFR:JDK-8054978:java.time.Duration.parse() fails for negative duration with 0 seconds and nanos

2015-11-13 Thread Stephen Colebourne
Looks good to me. thanks Stephen On 13 November 2015 at 11:03, nadeesh tv wrote: > Hi all, > > Please review a fix for Bug Id - > https://bugs.openjdk.java.net/browse/JDK-8054978 > > Issue - While parsing a negative Duration with 0 seconds and a few > nanoseconds, the

Re: RFR:JDK-8072746:LocalDate.isEra() should return IsoEra not Era

2015-11-13 Thread Stephen Colebourne
I'm happy with the webrev, however I'd like to see an official ruling as to whether this is an acceptable change to the spec. thanks Stephen On 13 November 2015 at 11:06, nadeesh tv wrote: > Hi all, > > Please review a fix for BugId - >

Re: RFR:JDK-8133079:java.time LocalDate and LocalTime ofInstant() factory methods

2015-11-12 Thread Stephen Colebourne
Looks good to me. Need to ensure that JDK-8030864 also gets tackled, as it is the inverse operation of these new methods. thanks Stephen On 12 November 2015 at 06:40, nadeesh tv wrote: > Hi all, > > Please review a fix for > > Bug Id -

Re: RFR:JDK-8138664- ZonedDateTime parse error for any date using 'GMT0' ZoneID

2015-11-10 Thread Stephen Colebourne
Good fix, thanks Stephen On 9 November 2015 at 20:36, Roger Riggs wrote: > Hi Nadeesh, > > Thanks for fixing this issue. > > It looks fine, I can sponsor the change for you. > > Roger > > > > On 11/9/2015 4:12 AM, nadeesh tv wrote: >> >> Hi all, >> >> Please review a fix

Re: RFR:JDK-8066571:UnsupportedTemporalTypeException is thrown not only in the case of unsupported temporal

2015-11-10 Thread Stephen Colebourne
Nice ti see this fixed thanks Stephen On 9 November 2015 at 21:34, Roger Riggs wrote: > Hi Naddesh, > > Thanks for fixing this issue. > > I can sponsor getting the fix pushed. > > Roger > > > > > On 11/9/2015 4:16 AM, nadeesh tv wrote: > > Hi all, > > Please review a fix

Re: 8141652 : Rename methods Objects.nonNullElse* to requireNonNullElse*

2015-11-06 Thread Stephen Colebourne
Seems fine to me. I would have inlined the ZoneId change to one line: String id = Objects.requireNonNullElse(aliasMap.get(zoneId), zoneId); to avoid the local variable change, but no big deal. Stephen On 6 November 2015 at 20:24, Roger Riggs wrote: > Please review the

Re: RFR: updated draft API for JEP 269 Convenience Collection Factories

2015-11-06 Thread Stephen Colebourne
I haven't reviewed in detail, but it seems like the right set of methods. There may be a case for adding equivalent to Queue, Deque, NavigableSet and SortedSet, if not now maybe in the future, although their usage is markedly lower. Stephen On 6 November 2015 at 02:13, Stuart Marks

Re: RFR 9: 8138963 : java.lang.Objects new method to default to non-null

2015-11-01 Thread Stephen Colebourne
On 31 October 2015 at 20:29, Roger Riggs wrote: > And, going back nearly to the beginning of the thread, some folks are > familiar with > > T firstNonNull(T, T) proposed from Guava. > > But since the exercise was turned into simplify: (x != null) ? x : >

Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-17 Thread Stephen Colebourne
On 14 October 2015 at 18:56, Kevin Bourrillion wrote: > Note that we have empirically learned through our Lists/Sets/Maps factory > classes that varargs factory methods for mutable collections are almost > entirely useless. Having taken a few days to think it over, I

Re: RFR: JDK-8046565: Platform Logger API and Service

2015-10-16 Thread Stephen Colebourne
On 15 October 2015 at 17:31, Daniel Fuchs wrote: >> I have a major concern that the class names 'Logger' and 'Level' >> duplicate those of java.util.logging. While they are inner classes as >> opposed to top level classes, both IntelliJ and Eclipse will find the >> inner

Re: RFR: JDK-8046565: Platform Logger API and Service

2015-10-15 Thread Stephen Colebourne
On 14 October 2015 at 12:20, Daniel Fuchs wrote: > On 14/10/15 07:21, Mandy Chung wrote: >> Several log methods throws NPE if the level is legible and the parameter >> is null. E.g. >> * @throws NullPointerException if {@code level} is {@code null}, or if the >> level >>

Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-15 Thread Stephen Colebourne
iff-2a9787868cf0654b4a6a07e75c1a6286R199 It occurs to me that it would be a *very* good idea to add a similar method to List. public static List of(int size, IntFunction valueFunction) { ... } Stephen On 14 October 2015 at 11:39, Stephen Colebourne <scolebou...@joda.org> wrote: > On 14 October 2015 at 10:38,

Re: Optional.or name Re: RFR 8080418 Add Optional.or()

2015-10-14 Thread Stephen Colebourne
Maybe "an optional equivalent to {@code this}" The original text is quite obtuse. Stephen On 14 October 2015 at 09:54, Paul Sandoz <paul.san...@oracle.com> wrote: > >> On 13 Oct 2015, at 22:43, Stephen Colebourne <scolebou...@joda.org> wrote: >>

Re: RFR:8134928:java.time.Instant.truncatedTo(TemporalUnit unit) is truncating up if the year < 1970

2015-10-14 Thread Stephen Colebourne
I'd like to see an additional test case for the cases where rounding is *not* needed. Currently, there are only tests for where rounding is needed. It needs one more test line for after 1970 and one for before 1970. thanks Stephen On 14 October 2015 at 10:53, nadeesh tv

Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-14 Thread Stephen Colebourne
On 14 October 2015 at 10:38, Paul Sandoz wrote: >> On 14 Oct 2015, at 06:18, Stuart Marks wrote: >> I'm not entirely sure what to take from this. If it were clearly >> exponential, we could say with confidence that above a certain threshold >>

Re: Optional.or name Re: RFR 8080418 Add Optional.or()

2015-10-13 Thread Stephen Colebourne
On 12 October 2015 at 08:48, Paul Sandoz wrote: > Now that the overall documentation changes and functional behaviour of > Optional.or has been agreed i would like to see if we can find a better name, > suggestions welcome, so lets get our paint brushes out :-) "or"

Re: RFR 9: 8138963 : java.lang.Objects new method to default to non-null

2015-10-09 Thread Stephen Colebourne
On 9 October 2015 at 01:31, John Rose wrote: > This leads me to yet another bikeshed color, FWIW: > > - T requireNonNull(T) (cf. Optional::get) > - T requireNonNullElse(T,T) (cf. Optional::orElse) > - T requireNonNullElseGet(T,Supplier) (cf. Optional::orElseGet) > - T

Re: RFC: draft API for JEP 269 Convenience Collection Factories

2015-10-09 Thread Stephen Colebourne
On 9 October 2015 at 00:39, Stuart Marks wrote: > 1. Number of fixed arg overloads. Guava follows this pattern: of(T) of(T, T) of(T, T, T) of(T, T, T, T... elements) whereas the proposal has of(T) of(T, T) of(T, T, T) of(T... elements) I'd be interested to know why

Re: RFR 9: 8138963 : java.lang.Objects new method to default to non-null

2015-10-07 Thread Stephen Colebourne
On 7 October 2015 at 23:24, Roger Riggs wrote: > Please consider and comment: > > T nonNullOf(T obj, T defaultObj); > T nonNullOf(T, obj, Supplier defaultSupplier); > > Details are in the updated webrev: >

<    1   2   3   4   5   >