You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/07/11 03:12:53 UTC
[GitHub] [commons-text] kinow commented on a change in pull request #248: TEXT-207: adding DoubleFormat utility
kinow commented on a change in pull request #248:
URL: https://github.com/apache/commons-text/pull/248#discussion_r667413903
##########
File path: src/main/java/org/apache/commons/text/numbers/DoubleFormat.java
##########
@@ -0,0 +1,732 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.text.numbers;
+
+import java.text.DecimalFormatSymbols;
+import java.util.Objects;
+import java.util.function.DoubleFunction;
+import java.util.function.Function;
+
+/** Enum containing standard double format types with methods to produce
+ * configured formatter instances. This type is intended to provide a
+ * quick and convenient way to create lightweight, thread-safe double format functions
+ * for common format types using a builder pattern.
+ *
+ * <p><strong>Comparison with DecimalFormat</strong>
+ * <p>This type provides some of the same functionality as Java's own
+ * {@link java.text.DecimalFormat}. However, unlike {@code DecimalFormat}, the format
+ * functions produced by this type are lightweight and thread-safe, making them
+ * much easier to work with in multi-threaded environments. They also provide performance
+ * comparable to, and in many cases faster than, {@code DecimalFormat}.
+ *
+ * <p><strong>Examples</strong>
Review comment:
Maybe before the examples, besides talking about this implementation & `DecimalFormat`, and thread-safety, also include a one-sentence long description for locale?
The examples contain an example with a `locale` variable, and some javadocs in the other class mention locale & thousands separators I think. Maybe worth describing how/if we will handle it too?
##########
File path: src/main/java/org/apache/commons/text/numbers/ParsedDecimal.java
##########
@@ -0,0 +1,725 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.text.numbers;
+
+/** Internal class representing a decimal value parsed into separate components. Each number
+ * is represented with
+ * <ul>
+ * <li>a boolean flag for the sign,</li>
+ * <li> a sequence of the digits {@code 0 - 10} representing an unsigned integer with leading and trailing zeros
+ * removed, and</li>
+ * <li>an exponent value that when applied to the base 10 digits produces a floating point value with the
+ * correct magnitude.</li>
+ * </ul>
+ * <p><strong>Examples</strong></p>
+ * <table>
+ * <tr><th>Double</th><th>Negative</th><th>Digits</th><th>Exponent</th></tr>
+ * <tr><td>0.0</td><td>false</td><td>[0]</td><td>0</td></tr>
+ * <tr><td>1.2</td><td>false</td><td>[1, 2]</td><td>-1</td></tr>
+ * <tr><td>-0.00971</td><td>true</td><td>[9, 7, 1]</td><td>-5</td></tr>
+ * <tr><td>56300</td><td>true</td><td>[5, 6, 3]</td><td>2</td></tr>
+ * </table>
+ */
+final class ParsedDecimal {
+
+ /** Interface containing values used during string formatting.
+ */
+ interface FormatOptions {
+
+ /** Return true if fraction placeholders (e.g., {@code ".0"} in {@code "1.0"})
+ * should be included.
+ * @return true if fraction placeholders should be included
+ */
+ boolean getIncludeFractionPlaceholder();
+
+ /** Return true if the string zero should be prefixed with the minus sign
+ * for negative zero values.
+ * @return true if the minus zero string should be allowed
+ */
+ boolean getSignedZero();
+
+ /** Get an array containing the localized digit characters 0-9 in that order.
+ * This string <em>must</em> be non-null and have a length of 10.
+ * @return array containing the digit characters 0-9
+ */
+ char[] getDigits();
+
+ /** Get the decimal separator character.
+ * @return decimal separator character
+ */
+ char getDecimalSeparator();
+
+ /** Get the character used to separate thousands groupings.
+ * @return character used to separate thousands groupings
+ */
+ char getGroupingSeparator();
+
+ /** Return true if thousands should be grouped.
+ * @return true if thousand should be grouped
+ */
+ boolean getGroupThousands();
+
+ /** Get the minus sign character.
+ * @return minus sign character
+ */
+ char getMinusSign();
+
+ /** Get the exponent separator as an array of characters.
+ * @return exponent separator as an array of characters
+ */
+ char[] getExponentSeparatorChars();
+
+ /** Return true if exponent values should always be included in
Review comment:
I normally use {@code true} to change the formatting in Javadoc, but not really important here.
##########
File path: pom.xml
##########
@@ -116,6 +119,22 @@
<version>${graalvm.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-rng-simple</artifactId>
+ <version>${commons.rng.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-core</artifactId>
+ <version>${jmh.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-generator-annprocess</artifactId>
+ <version>${jmh.version}</version>
Review comment:
Ditto this new dependency too?
##########
File path: pom.xml
##########
@@ -116,6 +119,22 @@
<version>${graalvm.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-rng-simple</artifactId>
+ <version>${commons.rng.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.openjdk.jmh</groupId>
+ <artifactId>jmh-core</artifactId>
+ <version>${jmh.version}</version>
Review comment:
Shouldn't it be scoped to test too? Or moved to the other profile perhaps?
##########
File path: src/main/java/org/apache/commons/text/numbers/ParsedDecimal.java
##########
@@ -0,0 +1,725 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.text.numbers;
+
+/** Internal class representing a decimal value parsed into separate components. Each number
+ * is represented with
+ * <ul>
+ * <li>a boolean flag for the sign,</li>
+ * <li> a sequence of the digits {@code 0 - 10} representing an unsigned integer with leading and trailing zeros
+ * removed, and</li>
+ * <li>an exponent value that when applied to the base 10 digits produces a floating point value with the
+ * correct magnitude.</li>
+ * </ul>
+ * <p><strong>Examples</strong></p>
+ * <table>
+ * <tr><th>Double</th><th>Negative</th><th>Digits</th><th>Exponent</th></tr>
+ * <tr><td>0.0</td><td>false</td><td>[0]</td><td>0</td></tr>
+ * <tr><td>1.2</td><td>false</td><td>[1, 2]</td><td>-1</td></tr>
+ * <tr><td>-0.00971</td><td>true</td><td>[9, 7, 1]</td><td>-5</td></tr>
+ * <tr><td>56300</td><td>true</td><td>[5, 6, 3]</td><td>2</td></tr>
+ * </table>
+ */
+final class ParsedDecimal {
+
+ /** Interface containing values used during string formatting.
+ */
+ interface FormatOptions {
+
+ /** Return true if fraction placeholders (e.g., {@code ".0"} in {@code "1.0"})
+ * should be included.
+ * @return true if fraction placeholders should be included
+ */
+ boolean getIncludeFractionPlaceholder();
+
+ /** Return true if the string zero should be prefixed with the minus sign
+ * for negative zero values.
+ * @return true if the minus zero string should be allowed
+ */
+ boolean getSignedZero();
+
+ /** Get an array containing the localized digit characters 0-9 in that order.
+ * This string <em>must</em> be non-null and have a length of 10.
+ * @return array containing the digit characters 0-9
+ */
+ char[] getDigits();
+
+ /** Get the decimal separator character.
+ * @return decimal separator character
+ */
+ char getDecimalSeparator();
+
+ /** Get the character used to separate thousands groupings.
+ * @return character used to separate thousands groupings
+ */
+ char getGroupingSeparator();
+
+ /** Return true if thousands should be grouped.
+ * @return true if thousand should be grouped
+ */
+ boolean getGroupThousands();
+
+ /** Get the minus sign character.
+ * @return minus sign character
+ */
+ char getMinusSign();
+
+ /** Get the exponent separator as an array of characters.
+ * @return exponent separator as an array of characters
+ */
+ char[] getExponentSeparatorChars();
+
+ /** Return true if exponent values should always be included in
+ * formatted output, even if the value is zero.
+ * @return true if exponent values should always be included
+ */
+ boolean getAlwaysIncludeExponent();
+ }
+
+ /** Minus sign character. */
+ private static final char MINUS_CHAR = '-';
+
+ /** Decimal separator character. */
+ private static final char DECIMAL_SEP_CHAR = '.';
+
+ /** Exponent character. */
+ private static final char EXPONENT_CHAR = 'E';
+
+ /** Zero digit character. */
+ private static final char ZERO_CHAR = '0';
+
+ /** Number of characters in thousands groupings. */
+ private static final int THOUSANDS_GROUP_SIZE = 3;
+
+ /** Radix for decimal numbers. */
+ private static final int DECIMAL_RADIX = 10;
+
+ /** Center value used when rounding. */
+ private static final int ROUND_CENTER = DECIMAL_RADIX / 2;
+
+ /** Number that exponents in engineering format must be a multiple of. */
+ private static final int ENG_EXPONENT_MOD = 3;
+
+ /** True if the value is negative. */
+ final boolean negative;
+
+ /** Array containing the significant decimal digits for the value. */
+ final int[] digits;
+
+ /** Number of digits used in the digits array; not necessarily equal to the length. */
+ int digitCount;
+
+ /** Exponent for the value. */
+ int exponent;
+
+ /** Output buffer for use in creating string representations. */
+ private char[] outputChars;
+
+ /** Output buffer index. */
+ private int outputIdx;
+
+ /** Construct a new instance from its parts.
+ * @param negative true if the value is negative
+ * @param digits array containing significant digits
+ * @param digitCount number of digits used from the {@code digits} array
+ * @param exponent exponent value
+ */
+ ParsedDecimal(final boolean negative, final int[] digits, final int digitCount,
+ final int exponent) {
+ this.negative = negative;
+ this.digits = digits;
Review comment:
I think in Imaging I got SpotBug warnings now for storing arrays directly (will check later whether that could affect text if we update spotbugs, or if we have an exclusion for that, or if CI is not running it, etc)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org