Re: RFR: JDK-8167618: DateTimeFormatter.format() uses exceptions for flow control.

[Anubhav Meena]

Thanks for the review!

Here is the updated webrev 
http://cr.openjdk.java.net/~ntv/others/anubhav/webrev.01/ 
<http://cr.openjdk.java.net/~ntv/others/anubhav/webrev.01/>

-Anubhav


> On Nov 2, 2016, at 4:04 PM, Stephen Colebourne <scolebou...@joda.org> wrote:
> 
> I agree with Nadeesh, the updated code can still throw
> DateTimeException from the call to getLong(). Unlike before, this
> DateTimeException is desired.
> 
> Stephen
> 
> 
> On 28 October 2016 at 16:58, nadeesh tv <nadeesh...@oracle.com> wrote:
>> 
>> Hi Anubhav,
>> 
>> 
>> - * @throws DateTimeException if the field is not available and the
>> section is not optional
>> 
>> 
>> I think you should not remove the DTException since still there is a chance
>> of throwing DTE
>> Regards,
>> Nadeesh
>> 
>> On 10/28/2016 12:18 AM, Anubhav Meena wrote:
>>> 
>>> Hi all,
>>> 
>>> Please review. Please ignore last mail as links not working properly
>>> there.
>>> 
>>> Bug Id : https://bugs.openjdk.java.net/browse/JDK-8167618
>>> <https://bugs.openjdk.java.net/browse/JDK-8167618>
>>> Issue:  DateTimeFormatter.format() uses exceptions for flow control.
>>> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/
>>> <http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/>
>> 
>> 
>> --
>> Thanks and Regards,
>> Nadeesh TV
>> 

RFR: JDK-8167618: DateTimeFormatter.format() uses exceptions for flow control.

[Anubhav Meena]

Hi all,

Please review. Please ignore last mail as links not working properly there.

Bug Id : https://bugs.openjdk.java.net/browse/JDK-8167618 

Issue:  DateTimeFormatter.format() uses exceptions for flow control.
Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/ 

-- 
Thanks and Regards,
Anubhav

RFR: JDK-8167618: DateTimeFormatter.format() uses exceptions for flow control.

[Anubhav Meena]

Hi all,

Please review.
Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8167618 

Issue:  DateTimeFormatter.format() uses exceptions for flow control.
Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8167618/webrev.00/ 

-- 
Thanks and Regards,
Anubhav

Re: RFR:JDK-8163330:HijrahDate aligned day of week incorrect

[Anubhav Meena]

Thanks Roger, appreciate that.

> On Oct 13, 2016, at 3:09 AM, Roger Riggs <roger.ri...@oracle.com> wrote:
> 
> Hi,
> 
> Looks ok.
> 
> Typically, there is a space after the '//' comment characters, it makes it 
> easier to read.
> Also, in this case, I don't think the comments help.  "Monday" isn't 
> important to the test 
> and the 'Any Other day' is still 1, not something else.
> 
> I would remove those comments.
> Otherwise fine, no need for another webrev.
> 
> Thanks, Roger
> 
> 
> On 10/12/2016 2:16 PM, Anubhav Meena wrote:
>> Hi Roger,
>> 
>> Thanks for the suggestion, have made the suggested changes and shifted the 
>> tests to /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java. 
>> 
>> Updated webrev: 
>> http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.04/ 
>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.04/>
>> 
>> Thanks,
>> Anubhav
>> 
>>> On Oct 12, 2016, at 8:45 PM, Roger Riggs <roger.ri...@oracle.com 
>>> <mailto:roger.ri...@oracle.com>> wrote:
>>> 
>>> Hi Anubhav,
>>> 
>>> In general, I agree with Stephen that the tests should be testing an 
>>> algorithm against facts.
>>> Embedding an algorithm in the test increases the risk that the test will 
>>> just replicate the 
>>> implementation code and therefor not be much of a test.
>>> 
>>> Though in this case, the specification of aligned day of week is of a 
>>> computation.
>>> If the test were to independently compute the correct answer, it would be 
>>> valid as a 'tck' test.
>>> 
>>> Since the Hijrah calendar is data driven, the tests should be in 
>>> /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java.
>>> Tests in java/time/tck/... should correspond directly to specified 
>>> behaviors.
>>> In this case, the algorithm is specified but the test is data dependent. 
>>> (Perhaps a gray area).
>>> 
>>> Thanks, Roger
>>> 
>>> 
>>> On 10/12/2016 11:03 AM, Anubhav Meena wrote:
>>>> Hi Stephen,
>>>> 
>>>> Have incorporated the changes you suggested. Updated webrev is available 
>>>> here 
>>>> http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/ 
>>>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.03/>
>>>> 
>>>> Please review and suggest if anymore changes are required.
>>>> 
>>>> Thanks,
>>>> Anubhav
>>>> 
>>>>> On Oct 12, 2016, at 3:21 PM, Anubhav Meena <anubhav.me...@oracle.com 
>>>>> <mailto:anubhav.me...@oracle.com>> wrote:
>>>>> 
>>>>> Gentle reminder.
>>>>> 
>>>>>> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com 
>>>>>> <mailto:anubhav.me...@oracle.com>> wrote:
>>>>>> 
>>>>>> Hi all,
>>>>>> 
>>>>>> Please review.
>>>>>> Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8163330>
>>>>>> 
>>>>>> Issue:  The HijrahDate class incorrectly calculates the 
>>>>>> aligned-day-of-week field. It based the calculation on the day-of-week, 
>>>>>> when it should be based on the day-of-month.
>>>>>> 
>>>>>> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 
>>>>>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.02/>
>>>>>> -- 
>>>>>> Thanks and Regards,
>>>>>> Anubhav
>>>>> 
>>>> 
>>> 
>> 
> 

Re: RFR:JDK-8163330:HijrahDate aligned day of week incorrect

[Anubhav Meena]

Hi Roger,

Thanks for the suggestion, have made the suggested changes and shifted the 
tests to /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java. 

Updated webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.04/

Thanks,
Anubhav

> On Oct 12, 2016, at 8:45 PM, Roger Riggs <roger.ri...@oracle.com> wrote:
> 
> Hi Anubhav,
> 
> In general, I agree with Stephen that the tests should be testing an 
> algorithm against facts.
> Embedding an algorithm in the test increases the risk that the test will just 
> replicate the 
> implementation code and therefor not be much of a test.
> 
> Though in this case, the specification of aligned day of week is of a 
> computation.
> If the test were to independently compute the correct answer, it would be 
> valid as a 'tck' test.
> 
> Since the Hijrah calendar is data driven, the tests should be in 
> /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java.
> Tests in java/time/tck/... should correspond directly to specified behaviors.
> In this case, the algorithm is specified but the test is data dependent. 
> (Perhaps a gray area).
> 
> Thanks, Roger
> 
> 
> On 10/12/2016 11:03 AM, Anubhav Meena wrote:
>> Hi Stephen,
>> 
>> Have incorporated the changes you suggested. Updated webrev is available 
>> here 
>> http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/ 
>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.03/>
>> 
>> Please review and suggest if anymore changes are required.
>> 
>> Thanks,
>> Anubhav
>> 
>>> On Oct 12, 2016, at 3:21 PM, Anubhav Meena <anubhav.me...@oracle.com 
>>> <mailto:anubhav.me...@oracle.com>> wrote:
>>> 
>>> Gentle reminder.
>>> 
>>>> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com 
>>>> <mailto:anubhav.me...@oracle.com>> wrote:
>>>> 
>>>> Hi all,
>>>> 
>>>> Please review.
>>>> Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8163330>
>>>> 
>>>> Issue:  The HijrahDate class incorrectly calculates the 
>>>> aligned-day-of-week field. It based the calculation on the day-of-week, 
>>>> when it should be based on the day-of-month.
>>>> 
>>>> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 
>>>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.02/>
>>>> -- 
>>>> Thanks and Regards,
>>>> Anubhav
>>> 
>> 
> 

Re: RFR:JDK-8163330:HijrahDate aligned day of week incorrect

[Anubhav Meena]

Hi Stephen,

Have incorporated the changes you suggested. Updated webrev is available here 
http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/

Please review and suggest if anymore changes are required.

Thanks,
Anubhav

> On Oct 12, 2016, at 3:21 PM, Anubhav Meena <anubhav.me...@oracle.com> wrote:
> 
> Gentle reminder.
> 
>> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com 
>> <mailto:anubhav.me...@oracle.com>> wrote:
>> 
>> Hi all,
>> 
>> Please review.
>> Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 
>> <https://bugs.openjdk.java.net/browse/JDK-8163330>
>> 
>> Issue:  The HijrahDate class incorrectly calculates the aligned-day-of-week 
>> field. It based the calculation on the day-of-week, when it should be based 
>> on the day-of-month.
>> 
>> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 
>> <http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/>
>> -- 
>> Thanks and Regards,
>> Anubhav
> 

Re: RFR:JDK-8163330:HijrahDate aligned day of week incorrect

[Anubhav Meena]

Gentle reminder.

> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com> wrote:
> 
> Hi all,
> 
> Please review.
> Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 
> <https://bugs.openjdk.java.net/browse/JDK-8163330>
> 
> Issue:  The HijrahDate class incorrectly calculates the aligned-day-of-week 
> field. It based the calculation on the day-of-week, when it should be based 
> on the day-of-month.
> 
> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 
> <http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/>
> 
> -- 
> Thanks and Regards,
> Anubhav

Reminder

[Anubhav Meena]

Hi All,

This is a reminder for the pending review.

> Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 
> <https://bugs.openjdk.java.net/browse/JDK-8163330>
> 
> Issue:  The HijrahDate class incorrectly calculates the aligned-day-of-week 
> field. It based the calculation on the day-of-week, when it should be based 
> on the day-of-month.
> 
> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 
> <http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/>

Thanks,
Anubhav


> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com> wrote:
> 
> Hi all,
> 
> Please review.
> Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 
> <https://bugs.openjdk.java.net/browse/JDK-8163330>
> 
> Issue:  The HijrahDate class incorrectly calculates the aligned-day-of-week 
> field. It based the calculation on the day-of-week, when it should be based 
> on the day-of-month.
> 
> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 
> <http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/>
> 
> -- 
> Thanks and Regards,
> Anubhav

RFR:JDK-8163330:HijrahDate aligned day of week incorrect

[Anubhav Meena]

Hi all,

Please review.

Bug Id :  https://bugs.openjdk.java.net/browse/JDK-8163330 


Issue:  The HijrahDate class incorrectly calculates the aligned-day-of-week 
field. It based the calculation on the day-of-week, when it should be based on 
the day-of-month.

Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ 



-- 
Thanks and Regards,
Anubhav