RFR (S) 8136500: Integer/Long getChars and stringSize should be more idiomatic

Aleksey Shipilev aleksey.shipilev at oracle.com
Mon Nov 23 18:05:40 UTC 2015


On 11/23/2015 08:58 PM, John Rose wrote:
> On Nov 23, 2015, at 8:23 AM, Ivan Gerasimov <ivan.gerasimov at oracle.com
> <mailto:ivan.gerasimov at oracle.com>> wrote:
>>
>> Though, it may be better to get yet another pair of eyes.
>>
>> One minor nit: In the tests, in the summary, it is written, "Test
>> Integer.toString method*s*", but only one of the overloads is tested.
> 
> Here's another nit in the tests.
> This is supposed to "wiggle around" critical points, which I agree with.
> But it only wiggles above:
> 
>   39         while (base < Long.MAX_VALUE / 10) {
>   40             for (int c = 1; c < 65536; c++) {
>   41                 buildAndTest(base + c);
>   42             }
>   43             base = (base == 0) ? 1 : base * 10;
>   44         }

Yes, it *does* wiggle around:

  71         test(sbN.toString(), -c);
  72         test(sbP.toString(), c);

This saves much testing time to build both "c" and "-c" representations.

> You'll need to guard the call to buildAndTest to avoid negatives.

It already does:

  51     private static void buildAndTest(int c) {
  52         if (c < 0) {
  53             throw new IllegalArgumentException("Test bug: cannot
handle negatives, " + c);
  54         }

> You could also start base at, say, 10000.

I don't see a reason for this, why? The overlap in the test data is
fine, if that makes test more understandable and therefore reliable.

Thanks,
-Aleksey





More information about the core-libs-dev mailing list