On Thu, 2 Nov 2023 22:21:15 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Restored ordering of exceptions.
>
> src/java.base/share/classes/java/math/BigInteger.java line 344:
>
>> 342: * @param len the number of bytes to use.
>> 343: * @throws NumberFormatException {@code val} is zero bytes long,
>> 344: * or {@code len} is zero.
>
> I would be concerned about the compatibility impact on existing applications.
> Though (very) uncommon, an existing application be using a non-zero length
> array and a len of zero to represent zero.
> With this change it will throw an unexpected exception.
A zero `len` is meant to indicate an empty range, whether the `val` array is
empty or not.
So, from a logical perspective, both cases `val.length == 0` or `len == 0`
should either both throw, or both should return `BigInteger` 0.
I choose the former, but the latter is a consistent choice as well.
What I find unpleasant is the current behavior: it throws on `val.length == 0`
(even if `len == 0`), but curiously returns 0 when it is non-zero and `len ==
0`. I find this rather inconsistent.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16449#discussion_r1381325977
