commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rob Tompkins <chtom...@gmail.com>
Subject Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently
Date Tue, 06 Sep 2016 12:17:32 GMT
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 <chtompki@gmail.com> wrote:
> 
> 
>> On Aug 18, 2016, at 3:10 AM, Jochen Wiedmann <jochen.wiedmann@gmail.com> wrote:
>> 
>> On Tue, Aug 16, 2016 at 9:10 PM, Benedikt Ritter <britter@apache.org> wrote:
>> 
>>>>> On Jul 31, 2016, at 3:03 PM, Rob Tompkins <chtompki@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
>> 
> 


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message