Hi Brian,
Pushed with recommend changes; webrev of final version:
http://cr.openjdk.java.net/~darcy/8233452.5
Thanks,
-Joe
On 1/14/2020 1:59 PM, Brian Burkhalter wrote:
Hi Joe,
This looks good modulo a few picayune things I noticed in the
implementation and test. Line numbers refer to the updated version of
each file.
1. Implementation
2: Newer copyright year 2020, of course.
Will do.
2150: s/much many/many/.
2185: I suppose that this is in case targetPrecision overflowed.
Right; I'll add a comment to make that explicit.
The code as written is probably not fully overflow-hardened, but I at
least wanted to avoid certain infinite loops from coming up.
2231: Why ulp = ulp?
Just a typo; will fix before pushing.
2376: s/larger than/larger/
2. Test
2: Newer copyright year 2020, of course.
39: Perhaps for symmetry with BigInteger
(https://bugs.openjdk.java.net/browse/JDK-8152237) we should add
BigDecimal.TWO at a later date.
112: s/valueOf(2)/TWO/
117: Alignment of RoundingMode.DOWN
137-138:
Probe inputs with two digits of precision, 10 … 99 and those
values scaled by 10^-1, 1.0, … 9.9, and by 10^-2, 0.1, … 0.99.
Corrected.
167: Two spaces after return
282, 303: “doesn’t not improperly” sounds awkward: should be “does not
improperly”?
289, 310: “Sqrt root” also sounds awkward
296, 317: Extra space before “:”
316: s/down/up/ ?
Yep; cut and paste issue.
337: s/rounding down/half even/ ?
362: s/near 1 half even/“near 1 “ + mc.toString()/ ?
397: delete commented out line?
520: s/+1)/+ 1)/
607: Introduce a square() method in BigSquareRoot also for parity with
BigDecimal?
Addressed.
