commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently
Date Tue, 16 Aug 2016 19:10:51 GMT
Hello Rob,

I'm back from vacation and I will have a look at this issue again later
this week.

Benedikt

Rob Tompkins <chtompki@gmail.com> schrieb am Do., 11. Aug. 2016 um
16:48 Uhr:

>
> > 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