Please add @since 1.9 to the new constructors.
Cheers,
Paul
On Tue, Dec 30, 2014 at 8:15 PM, joe darcy <joe.da...@oracle.com> wrote:
> Hi Brian,
>
> The new changes generally look good. A few comments, for the new code like
>
> 291 } else if ((off < 0) || (off > val.length) || (len < 0) ||
> 292 ((off + len) > val.length) || ((off + len) < 0)) {
> 293 throw new IndexOutOfBoundsException();
>
> it is not immediately clear that the arithmetic on line 292 won't have
> some inappropriate overflow behavior. A comment stating why "off + len"
> will behave appropriately (assuming it does behave appropriately ;-) would
> help. (By line 292, both off and len are non-negative so that should limit
> the case analysis.)
>
> It might be worthwhile for all the BigInteger constructors which take
> array arguments to state something about the thread-safely behavior
> ("arrays are assumed to be unchanged for the duration of the constructor
> call...").
>
> Do have have any code coverage information for the new code by the
> regression test? It would be good to know whether or not all the guard
> conditions are properly being executed.
>
> Thanks,
>
> -Joe
>
>
> On 12/30/2014 8:33 AM, Brian Burkhalter wrote:
>
>> I’ve added the suggested @throws tags here:
>>
>> http://cr.openjdk.java.net/~bpb/4026465/webrev.01/
>>
>> Thanks,
>>
>> Brian
>>
>> On Dec 30, 2014, at 2:33 AM, Stephen Colebourne <scolebou...@joda.org>
>> wrote:
>>
>> Just to note that an IndexOutOfBoundsException is mentioned in the
>>> text (line 274, 350) but there is no matching @throws clause. The
>>> IndexOutOfBoundsException could have a message added.
>>>
>>> In general, I would expect an offset/length overload for most methods
>>> that take an array, so this seems like a sensible request.
>>>
>>
>
