Hi Naoto,

The CSR looks ok.

In the implementation ValueRange.of: 148-150; I would omit the check here since
it invokes the 4 arg form and does the same check.
The difference in exception message is not worth the extra code.
Though it would help if it said that the 3 arg form is the same as the 4 arg form with the same first two args.

In the test TestDateTimeValueRange.java, the separate test case is (mostly) redundant with the added case at line 187. (Though that assumes the 3 arg form calls the 4 arg form; as does the lack of any other testing of the 3 arg form).

Also,
It might be useful to verify that each of the test cases 179-186 trigger only 1 of the range checks. If a test case would fail more than 1 of the conditions then it might be never be testing each if statements.

Thanks, Roger


On 2/20/20 5:28 PM, naoto.s...@oracle.com wrote:
Hi,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8239520

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8239520/webrev.00/

This change lets 3-arg ValueRange.of() method thrown an IAE on invalid inputs, as well as corrects the method description of 4-arg ValueRange.of() method which lacks an IAE condition. I also filed a CSR for this change accordingly:

https://bugs.openjdk.java.net/browse/JDK-8239574

Naoto

Reply via email to