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 Thu, 11 Aug 2016 14:47:45 GMT

> On Jul 31, 2016, at 3:03 PM, Rob Tompkins <chtompki@gmail.com> wrote:
> 
> I figured that I would run an analogous test to the original with isParsable and createNumber,
and I ran into the following:
> 
> 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’m guessing then that we would want to pick either “isParsable(str)” or “isNumber(str)”
to track along with the wider range encapsulated in “createNumber(str),” which, based
upon the conversation below, I would guess would be “isParsable(str).”

My question now is: should I go ahead and incorporate that into LANG-1252 with the namespace
change/deprecation given below or should that go into a new issue?

Cheers,
-Rob

> I’m not sure if we should tackle that in this Jira issue. What do you guys think?
> 
> -Rob
> 
>> On Jul 31, 2016, at 6:23 AM, Benedikt Ritter <britter@apache.org <mailto:britter@apache.org>>
wrote:
>> 
>> How about deprecating and renaming?
>> 
>> isNumber -> isJavaNumberLiteral
>> createNumber -> parseNumber
>> 
>> this way there would be a clearer connection between isParsebale and the
>> method that parses the number. Furthermore it is clearer what isNumber
>> really does.
>> 
>> Benedikt
>> 
>> Rob Tompkins <chtompki@gmail.com <mailto:chtompki@gmail.com>> schrieb
am Sa., 30. Juli 2016 um 21:17:
>> 
>>> 
>>>> On Jul 30, 2016, at 12:41 PM, Gary Gregory <garydgregory@gmail.com <mailto:garydgregory@gmail.com>>
>>> wrote:
>>>> 
>>>> Can this all be solved with better Javadocs?
>>>> 
>>> 
>>> I don’t think so. So, I would think that we would do one of two things:
>>> 
>>> (1) Clarify Javadocs, and set up unit tests to reflect the intent (that
>>> being that createNumber and isNumber as slightly different)
>>> 
>>> or
>>> 
>>> (2) Refactor the code so that they use the same checks to either return
>>> the boolean or create the number.
>>> 
>>> Personally I’m indifferent, but I think those are the two different tacks
>>> we can take.
>>> 
>>> -Rob
>>> 
>>>> Gary
>>>> 
>>>> On Jul 29, 2016 7:59 AM, "Rob Tompkins" <chtompki@gmail.com <mailto:chtompki@gmail.com>>
wrote:
>>>> 
>>>>> Hi Benedikt,
>>>>> 
>>>>> Thanks for the insights here.
>>>>> 
>>>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <britter@apache.org
<mailto:britter@apache.org>>
>>> wrote:
>>>>>> 
>>>>>> Hello Rob,
>>>>>> 
>>>>>> Rob Tompkins <chtompki@gmail.com <mailto:chtompki@gmail.com>
<mailto:chtompki@gmail.com <mailto:chtompki@gmail.com>>> schrieb
>>>>> am Do., 28. Juli 2016 um
>>>>>> 14:23 Uhr:
>>>>>> 
>>>>>>> In short, I’m trying to generalize LANG-1060, LANG-1040, LANG-1038,
>>> and
>>>>>>> LANG-992 with a single issue that actually hits all the bases
here
>>> with
>>>>>>> NumberUtils.isNumber.
>>>>>>> 
>>>>>>> Bug (1):
>>>>>>> 
>>>>>>>      System.out.println(lang.math.NumberUtils.isNumber(“+2”));
---->
>>>>>>> false
>>>>>>> 
>>>>>>> while
>>>>>>> 
>>>>>>>      System.out.println(lang.math.NumberUtils.createNumber(“+2));
>>>>> ---->
>>>>>>> 2
>>>>>>> 
>>>>>>> Bug (2):
>>>>>>> 
>>>>>>> 
>>>>>>>      System.out.println(lang.math.NumberUtils.isNumber(“01.5”));
>>> ---->
>>>>>>> false
>>>>>>> 
>>>>>>> while
>>>>>>> 
>>>>>>>      System.out.println(lang.math.NumberUtils.createNumber(“01.5));
>>>>>>> ----> 1.5.
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> It seems to me that we could externalize a considerable amount
of the
>>>>> code
>>>>>>> underlying the two methods into shared methods, as it seems like
all
>>> the
>>>>>>> validations in createNumber that predicate object creation should
be
>>>>>>> directly used in isNumber. I would love to hear folks’ thoughts.
>>>>>>> 
>>>>>> 
>>>>>> I think it is important to pay close attention to the JavaDocs in
this
>>>>> case.
>>>>>> 
>>>>>> NumberUtils.isNumber :
>>>>>> Checks whether the String a valid Java number.
>>>>>> 
>>>>>> This has nothing to do with createNumber:
>>>>>> NumberUtils.createNumber: Turns a string value into a java.lang.Number
>>>>>> 
>>>>>> What you're probably looking for is:
>>>>>> 
>>>>>> NumberUtils.isParsebale: Checks whether the given String is a parsable
>>>>>> number.
>>>>>> 
>>>>>> The difference is, that isNumber tells you, whether putting the given
>>>>>> String into Java code woul compile, while isParseble stells you
>>> whether a
>>>>>> call to createNumber will be successful.
>>>>> 
>>>>> This feels confusing to me (mainly because I’m not clear of the context
>>> of
>>>>> the word “compile” here). But, I’m mostly agnostic regarding intent
>>> here.
>>>>> So let’s not concern ourselves with that.
>>>>> 
>>>>> Maybe the Jira issue that I created, LANG-1252, should just make the
>>>>> javadoc clearer in this case.
>>>>> 
>>>>> Along the same line of thought, in NumberUtilsTest
>>>>>       testIsNumber()
>>>>> calls the function
>>>>> private void compareIsNumberWithCreateNumber(final String val, final
>>>>> boolean expected) {
>>>>>   final boolean isValid = NumberUtils.isNumber(val);
>>>>>   final boolean canCreate = checkCreateNumber(val);
>>>>>   if (isValid == expected && canCreate == expected) {
>>>>>       return;
>>>>>   }
>>>>>   fail("Expecting "+ expected + " for isNumber/createNumber using \""
+
>>>>> val + "\" but got " + isValid + " and " + canCreate);
>>>>> }
>>>>> where “checkCreateNumber” is declared as:
>>>>> private boolean checkCreateNumber(final String val) {
>>>>>   try {
>>>>>       final Object obj = NumberUtils.createNumber(val);
>>>>>       if (obj == null) {
>>>>>           return false;
>>>>>       }
>>>>>       return true;
>>>>>   } catch (final NumberFormatException e) {
>>>>>       return false;
>>>>>  }
>>>>> }
>>>>> which continues to puzzle me. If, indeed, it is the case that isNumber
>>> and
>>>>> createNumber necessarily differ, might we also refactor the unit tests
>>> as
>>>>> part of LANG-1252 such that they aren’t directly correlated for the
>>> sake of
>>>>> validation?
>>>>> 
>>>>> Again, many thanks for the response here.
>>>>> 
>>>>> Cheers,
>>>>> -Rob
>>>>> 
>>>>>> 
>>>>>> Regards,
>>>>>> Benedikt
>>>>>> 
>>>>>> 
>>>>>>> Cheers,
>>>>>>> -Rob
>>>>> 
>>>>> 
>>> 
>>> 
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org <mailto:dev-unsubscribe@commons.apache.org>
>>> For additional commands, e-mail: dev-help@commons.apache.org <mailto:dev-help@commons.apache.org>
>>> 
>>> 
> 


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