On Wed, 8 Nov 2023 19:24:24 GMT, Joe Darcy <da...@openjdk.org> wrote:
>> Add discussion of some of the common pitfalls related to decimal <-> binary >> conversion. > > Joe Darcy has updated the pull request incrementally with one additional > commit since the last revision: > > Switch to parseFloat from valueOf, add links. Left some minor proofreading comments. (Not a Reviewer) src/java.base/share/classes/java/lang/Double.java line 209: > 207: * decimal conversion. While integer values can be exactly represented > 208: * in any base, which fractional values can be exactly represented is > 209: * a function of the base. For example, in base 10, 1/3 is a repeating Could the first sentence be simplified and "de-aspectized"? Suggestion: "..trace back to conversions between binary and decimal representations". The second sentence has a suspicious while/which pairing and "represented is" should probably be "represented as". Suggestion: "While integer values can be exactly represented in any base, fractional values can only be exactly represented as a function of the base." src/java.base/share/classes/java/lang/Double.java line 243: > 241: * double}. Alternatives include using an integer type and storing > 242: * cents or mills or using {@link java.math.BigDecimal BigDecimal} to > 243: * store decimal fraction values exactly. The use of and/or here makes this sentence somewhat hard to parse. Perhaps a bit of punctuation could help readability? src/java.base/share/classes/java/lang/Double.java line 246: > 244: * > 245: * <p>For each finite floating-point value and a given floating-point > 246: * type, there is a contiguous region of the real number line which Neither English nor floating-point are native to me, but would it be better to use "region on the real number line" here? A quick Google search of the alternatives gives 4 results for "region of.." and 22.000 for "region on.." src/java.base/share/classes/java/lang/Double.java line 275: > 273: * } > 274: * > 275: * <p>An analogous range can be constructed similarly for the {@code Suggestion: * <p>Similarly, an analogous range can be constructed for the {@code src/java.base/share/classes/java/lang/Double.java line 289: > 287: * </ul> > 288: * > 289: * A floating-point value doesn't "know" if it was the result of Would it be better to use "whether" in this listing of alternatives? (As recommended by Merriam-Webster: https://www.merriam-webster.com/grammar/if-vs-whether-difference-usage) src/java.base/share/classes/java/lang/Double.java line 305: > 303: * > 304: * should <em>not</em> be expected to be exactly equal to 1.0, but > 305: * only close to 1.0. Consequently the following code is an infinite > loop: I think "Consequently" is a conjunctive adverb here, so should be followed by a comma. Or perhaps just drop it, since the previous sentence also starts with "Consequently, " src/java.base/share/classes/java/lang/Double.java line 314: > 312: * } > 313: * > 314: * Instead, for counted loops, use an integer loop count: Suggestion: * Instead, use integer loop counters for counted loops: ------------- PR Review: https://git.openjdk.org/jdk/pull/16566#pullrequestreview-1721041731 PR Review Comment: https://git.openjdk.org/jdk/pull/16566#discussion_r1387055262 PR Review Comment: https://git.openjdk.org/jdk/pull/16566#discussion_r1387060390 PR Review Comment: https://git.openjdk.org/jdk/pull/16566#discussion_r1387063710 PR Review Comment: https://git.openjdk.org/jdk/pull/16566#discussion_r1387092563 PR Review Comment: https://git.openjdk.org/jdk/pull/16566#discussion_r1387077414 PR Review Comment: https://git.openjdk.org/jdk/pull/16566#discussion_r1387086007 PR Review Comment: https://git.openjdk.org/jdk/pull/16566#discussion_r1387097101