-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NumberUtils countSignificantFigures method #1095
base: master
Are you sure you want to change the base?
Conversation
This cannot parse standard floating point numbers generated by String valueOf as the exponent character in that case is E. What if the exponent is negative? What if the number is negative? What if either number or exponent have an explicit + sign? What if the number has more than 1 decimal point? |
All good points. Code should be fixed to handle all of these successfully. Although an explicit '+' might debatably be an error case, I would think it's better to handle it and document that it's allowed.
Code should be fixed to fail on this case. |
@@ -32,6 +33,20 @@ | |||
*/ | |||
public class NumberUtils { | |||
|
|||
/** Reusable char constant for zero. */ | |||
public static final char CHAR_ZERO = '0'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are all superfluous. Named constants are for number, not string and char literals.
* Counts the number of significant figures within a decimal value string. | ||
* {@link org.apache.commons.lang3.math.NumberUtils#countSignificantFigures(String, char)} | ||
* | ||
* @param decimalNumber The decimal number string. Must be numeric digits with optional decimal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow Oracle guidelines for formatting doc comments
} | ||
|
||
/** | ||
* Counts the number of significant figures within a decimal value string. This method must |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need a link to an authoritati=ve definition o0f signifcant figures
|
||
/** | ||
* Counts the number of significant figures within a decimal value string. This method must | ||
* accept a string because significant figures can only be inferred from the a user provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete the
*/ | ||
public static int countSignificantFigures(String decimalNumber, char decimalSeparator) { | ||
if (decimalNumber.length() == 0) { | ||
throw new IllegalArgumentException("Decimal Number string was empty"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs a doc comment throws clause
if (CharUtils.isAsciiNumeric(decimalSeparator)) { | ||
throw new IllegalArgumentException("Decimal Separator cannot be numeric"); | ||
} | ||
if (decimalSeparator == CHAR_EXPONENT_UPPER || decimalSeparator == CHAR_EXPONENT_LOWER) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline the constants and I could read this without scrolling
A util method in NumberUtils that counts the number of significant figures in a decimal strings of standard / exponential format. e.g.(101, 102.1, 5.1e2, 1.001, 0.2e4, etc...)
This method counts the number of "significant figures" according to standard mathematical rules (details and references are in the javadoc). It must operate on a string because the rules themselves inherently depend on the string representation and not the underlying numeric value. The rules are well documented and widely accepted, aside from zero values, which are not usually covered by the conventions and the choice as to how to interpret is documented in the javadoc.
My first instinct was that this belonged in commons-math or commons-numbers, as the concept of significant figures is usually necessary in scientific / mathematical fields, but thus far it doesnt seem to be a good fit. Decided Id create the PR here so that at least the implementation is out there for review and perhaps this discussion will find it a better home. I do feel like this is a valuable feature because any manipulation of numerical scientific data that needs to be accurate (e.g. clinical data) ultimately needs to respect significant figure counts, which are not tracked by the standard java Number classes.
There are 7 basic rules that the method follows, No. 1-5 being the core sigfig rules that you'll find on most external sources, No. 6 to explain how/why zero values are handled, and No. 7 to explain how exponents are handled (or excluded, rather). The overall format of the string is validated, with IAEs thrown for invalid characters or formatting.
Examples of the results: