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 Sun, 31 Jul 2016 12:22:31 GMT


> On Jul 31, 2016, at 6:23 AM, Benedikt Ritter <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.

Sure I'll see if I can get a pull request together that heads this direction. 

Would you refactor the unit test for isJavaNumberLiteral to not use parseNumber?

-Rob


> 
> Benedikt
> 
> Rob Tompkins <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>
>>> 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> wrote:
>>>> 
>>>> Hi Benedikt,
>>>> 
>>>> Thanks for the insights here.
>>>> 
>>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <britter@apache.org>
>> wrote:
>>>>> 
>>>>> Hello Rob,
>>>>> 
>>>>> Rob Tompkins <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
>> For additional commands, e-mail: dev-help@commons.apache.org
>> 
>> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message