Hi Ivan, Please see the delta [1] and absolute [2] webrevs.
> On Aug 21, 2019, at 7:38 PM, Ivan Gerasimov <ivan.gerasi...@oracle.com> wrote: > > A couple of comments > > 1) > 3974 if (signum == 0) { > 3975 buf.append('0'); > 3976 return; > 3977 } > > Shouldn't it be padded with zeroes, if digits > 0? > If I'm not mistaken, it could happen if result[1] at the end of toString() > happens to be ZERO. Good catch: you are absolutely correct. > It that's true, then it may be a good idea to add a testcase for a number > with many trailing zeros in a string representation. Currently, > test/jdk/java/math/BigInteger/BigIntegerTest.java in stringConv() operates on > random numbers, which are unlikely to have huge number of trailing zeros. I added testing this to stringConv(). The test fails without the most recent delta applied to the implementation. > 2) > 4120 /* zero is a string of NUM_ZEROS consecutive zeros. */ > 4121 private static final String zeros = "0".repeat(NUM_ZEROS); > > Nit in the comment: name of the constant should be zeros. I changed the variable name to ZEROS as it is a constant. Thanks, Brian [1] http://cr.openjdk.java.net/~bpb/8229845/webrev.02-03/ [2] http://cr.openjdk.java.net/~bpb/8229845/webrev.03/