Hi Brian!
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.
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.
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 zero*s*.
With kind regards,
Ivan
On 8/21/19 5:28 PM, Brian Burkhalter wrote:
Hello,
On Aug 20, 2019, at 4:31 PM, Brian Burkhalter <brian.burkhal...@oracle.com>
wrote:
On Aug 20, 2019, at 2:45 PM, Ivan Gerasimov <ivan.gerasi...@oracle.com
<mailto:ivan.gerasi...@oracle.com>> wrote:
Would it make sense to add an argument `digits` to smallToString (like the same
named argument of toString, the minimum number of digits to pad to), so it
could zero-pad the result itself?
This way we could avoid inserting zeros into the middle of a StringBuilder
after a call to smallToString.
Indeed I do not like the zero insertion either. I’ll investigate your idea.
I modified the patch so that characters are exclusively appended to the
StringBuilder thereby eliminating copying of characters which are already
present after the insertion point, viz., the padding with zeros after calling
smallToString() in the recursive path. A delta versus version .01 is at [1] and
the diff versus the tip at [2].
Thanks,
Brian
[1] http://cr.openjdk.java.net/~bpb/8229845/webrev.01-02/
[2] http://cr.openjdk.java.net/~bpb/8229845/webrev.02/
--
With kind regards,
Ivan Gerasimov
