Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

[Rob Tompkins <ch...@gmail.com>]

I spent considerable time over the weekend here playing around with different ideas on how to accomplish the reconciliation of the methods at hand here in NumberUtils. The more I fiddle around with the issue, the more I really want to rename “isNumber,” “isParsable” and take “isNumber” (what would be “isJavaNumberLiteral”) and completely rewrite it.

I keep returning to this portion of comments:
https://github.com/apache/commons-lang/blob/master/src/test/java/org/apache/commons/lang3/math/NumberUtilsTest.java#L1218-L1223 <https://github.com/apache/commons-lang/blob/master/src/test/java/org/apache/commons/lang3/math/NumberUtilsTest.java#L1218-L1223>
/**
 * Tests isNumber(String) and tests that createNumber(String) returns
 * a valid number iff isNumber(String) returns false.
 */
@Test
public void testIsNumber() {


Further, to me, it feels like that isJavaNumberLiteral is not so far away from isParsable in terms of functionality. Clearly there would be some subtle differences, but they feel remarkably more similar than “isNumber” and “isParsable” are currently in the code base (with “isParsable” being substantially more simplistic).

The reason for another email to the M.L. here is: before getting too far into implementation, I wanted to bounce some of these ideas around again.

Implementation thoughts:
(1) Switch implementation “isNumber” -> “isParsable”
(2) Deprecate “isNumber” and have it call “isParsable"
(3) Rename “createNumber” to “parseNumber” (with Deprecation)
(4) Fix subtle differences between “parseNumber” and newly implemented “isParsable"
(5) Write “isJavaNumberLiteral” from beginning.

What do you guys think. Also, I don’t think I’d be comfortable making such extensive changes without some one like Benedikt putting another set of eyes on the work.

Cheers,
-Rob


> On Aug 19, 2016, at 4:53 PM, Rob Tompkins <ch...@gmail.com> wrote:
> 
> 
>> On Aug 18, 2016, at 3:10 AM, Jochen Wiedmann <jo...@gmail.com> wrote:
>> 
>> On Tue, Aug 16, 2016 at 9:10 PM, Benedikt Ritter <br...@apache.org> wrote:
>> 
>>>>> On Jul 31, 2016, at 3:03 PM, Rob Tompkins <ch...@gmail.com> wrote:
>> 
>>>>> System.out.println(NumberUtils.isParsable("+2")); ----> false
>>>>> System.out.println(NumberUtils.createNumber("+2")); ---> 2
>>>> If I had to guess the cause of this discrepancy, I would think that we
>>>> would want “isNumber(str)” and “isParsable(str)” to be as restrictive as
>>>> Java 1.6 for the sake of compatibility, noting that “+2” only can be parsed
>>>> to a Double or Float in Java 1.6. That said, I’m assuming that we want
>>>> “createNumber(str)” to hit a wider range of strings for number
>>>> instantiation.
>> 
>> I suggest to consider the following:
>> 
>> - Replace isParsable(String) with (or, add a new method)
>> getParseableType(String), where the latter would return something like
>>    "UNPARSEABLE", "FLOAT", "DOUBLE", indicating the expected result
>> type for createNumber(String).
> 
> Maybe something more along the lines of isParsable(String), returning true if the string is parsable to any subclasses of java.lang.Number, and then as our other method we would have something along the lines of isParsable(String,class) returning true if we can parse to that specific subclass of java.lang.Number.
> 
> Either way, should this work go under LANG-1252 or a different issue?
> 
> -Rob
> 
>> 
>> Jochen
>> 
>> -- 
>> The next time you hear: "Don't reinvent the wheel!"
>> 
>> http://www.keystonedevelopment.co.uk/wp-content/uploads/2014/10/evolution-of-the-wheel-300x85.jpg
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>> 
> 

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

[Eric Barnhill <er...@gmail.com>]

> Implementation thoughts:
> (1) Switch implementation “isNumber” -> “isParsable”
> (2) Deprecate “isNumber” and have it call “isParsable"
> (3) Rename “createNumber” to “parseNumber” (with Deprecation)
> (4) Fix subtle differences between “parseNumber” and newly implemented
> “isParsable"
> (5) Write “isJavaNumberLiteral” from beginning.
>


I took a look at the code. As far as design goes I think they are all good
improvements. I did notice this in the doc:

--
<p>Parsable numbers include those Strings understood by {@link
Integer#parseInt(String)},
     * {@link Long#parseLong(String)}, {@link Float#parseFloat(String)} or
     * {@link Double#parseDouble(String)}. This method can be used instead
of catching {@link java.text.ParseException}
     * when calling one of those methods.</p>
<p>Hexadecimal and scientific notations are <strong>not</strong> considered
parsable.
--

So this makes me wonder if a method called isParsable should in fact be
able to parse these numbers? Is this doc canonical somehow, or just how
previous contributors chose to think about the issue? In which case the doc
could be restated, saying for example "Hexadecimal and scientific notations
are <strong>not</strong> considered Number Literals."

Regarding point 5, this page

https://en.wikibooks.org/wiki/Java_Programming/Literals

uses the term "Numeric Literal", perhaps that is better phrasing than
"Number Literal"?

Eric