garydgregory commented on PR #216:
URL: https://github.com/apache/commons-cli/pull/216#issuecomment-1879728747
Hi @Claudenw
Hm, I think this PR is overly complicated.
Specifically, I do not think there should be a split between verifying and
converting, there should not be a whole `Verifier` infrastructure IMO: The
converters do their parsing, and in this PR, the syntaxes between verifying and
parsing don't match anyway.
For a simple example, it is legal for an Integer to have a leading `+`
character but the regular expression for integers doesn't allow it.
It's worse if I want to have a Double parameter since the RE for that is
large and complex as documented in `Double.valueOf(String)` and it's certainly
not what this PR does:
```
* final String Digits = "(\\p{Digit}+)";
* final String HexDigits = "(\\p{XDigit}+)";
* // an exponent is 'e' or 'E' followed by an optionally
* // signed decimal integer.
* final String Exp = "[eE][+-]?"+Digits;
* final String fpRegex =
* ("[\\x00-\\x20]*"+ // Optional leading "whitespace"
* "[+-]?(" + // Optional sign character
* "NaN|" + // "NaN" string
* "Infinity|" + // "Infinity" string
*
* // A decimal floating-point string representing a finite
positive
* // number without a leading sign has at most five basic pieces:
* // Digits . Digits ExponentPart FloatTypeSuffix
* //
* // Since this method allows integer-only strings as input
* // in addition to strings of floating-point literals, the
* // two sub-patterns below are simplifications of the grammar
* // productions from section 3.10.2 of
* // The Java Language Specification.
*
* // Digits ._opt Digits_opt ExponentPart_opt FloatTypeSuffix_opt
* "((("+Digits+"(\\.)?("+Digits+"?)("+Exp+")?)|"+
*
* // . Digits ExponentPart_opt FloatTypeSuffix_opt
* "(\\.("+Digits+")("+Exp+")?)|"+
*
* // Hexadecimal strings
* "((" +
* // 0[xX] HexDigits ._opt BinaryExponent FloatTypeSuffix_opt
* "(0[xX]" + HexDigits + "(\\.)?)|" +
*
* // 0[xX] HexDigits_opt . HexDigits BinaryExponent
FloatTypeSuffix_opt
* "(0[xX]" + HexDigits + "?(\\.)" + HexDigits + ")" +
*
* ")[pP][+-]?" + Digits + "))" +
* "[fFdD]?))" +
* "[\\x00-\\x20]*");// Optional trailing "whitespace"
*
* if (Pattern.matches(fpRegex, myString))
* Double.valueOf(myString); // Will not throw NumberFormatException
* else {
* // Perform suitable alternative action
* }
```
All of that to say, that you don't have to worry about any of this if you
let a converter do its usual parsing work without trying to short-circuit the
process with an _additional_ parsing step. IOW, don't parse input twice, we
don't need verifiers.
Nit: `@param` Javadoc tags should be listed first.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
