Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

Looks fine Ivan. Reviewed. Regards, Sean. On 19/08/16 15:59, Ivan Gerasimov wrote: On 19.08.2016 17:32, Stephen Colebourne wrote: I'm less comfortable with the compareTo change because it may make it slower and that may have knock on effects. I think a comment saying that both are bounded

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

On 19.08.2016 17:32, Stephen Colebourne wrote: I'm less comfortable with the compareTo change because it may make it slower and that may have knock on effects. I think a comment saying that both are bounded would be enough in compareTo() Okay. I reverted that change back then and added a

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

I'm less comfortable with the compareTo change because it may make it slower and that may have knock on effects. I think a comment saying that both are bounded would be enough in compareTo() Stephen On 19 August 2016 at 13:52, Ivan Gerasimov wrote: > Thanks Nadeesh.

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

Thanks Nadeesh. It's a good catch! Here's the updated webrev: http://cr.openjdk.java.net/~igerasim/8164366/01/webrev/ I also slightly modified comareTo(), not because there was an error in it, but just to avoid thinking too much about possible overflow in subtraction (of course, there can be

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

Hi Ivan, ZoneOffset.ofTotalSeconds(Integer.MIN_VALUE) have also the same issue. It' not throwing the expected DTE. May be you can correct this also as part of this. Please update the copyright year also. Rest looks good. -- Thanks and Regards, Nadeesh TV On 8/18/2016 10:23 PM, Ivan

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

Hi , Sorry , my bad. I misread 'plusmn'. -- Thanks and Regards, Nadeesh TV On 8/19/2016 11:10 AM, nadeesh tv wrote: Hi Stephen/Ivan, Is not the the statement "Zone offset minutes and seconds must be negative because hours is negative" and the following doc definition contradicts ? 358

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

Thank you Stephen for review! On 18.08.2016 21:16, Stephen Colebourne wrote: Looks good Stephen On 18 August 2016 at 17:53, Ivan Gerasimov wrote: Hello everybody! The factory methods of ZoneOffset class demonstrate a minor inconsistency. Normally, invalid values

Re: [jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

Looks good Stephen On 18 August 2016 at 17:53, Ivan Gerasimov wrote: > Hello everybody! > > The factory methods of ZoneOffset class demonstrate a minor inconsistency. > Normally, invalid values of arguments are rejected (e.g. when minutes > 59), > but the value of

[jdk9] RFR: 8164366: ZoneOffset.ofHoursMinutesSeconds() does not reject invalid input

Hello everybody! The factory methods of ZoneOffset class demonstrate a minor inconsistency. Normally, invalid values of arguments are rejected (e.g. when minutes > 59), but the value of Integer.MIN_VALUE is allowed to be passed in. This is due to using Math.abs(), which cannot handle