> On Jul 31, 2016, at 6:23 AM, Benedikt Ritter <brit...@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 <chtom...@gmail.com> schrieb am Sa., 30. Juli 2016 um 21:17: > >> >>>> On Jul 30, 2016, at 12:41 PM, Gary Gregory <garydgreg...@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" <chtom...@gmail.com> wrote: >>>> >>>> Hi Benedikt, >>>> >>>> Thanks for the insights here. >>>> >>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <brit...@apache.org> >> wrote: >>>>> >>>>> Hello Rob, >>>>> >>>>> Rob Tompkins <chtom...@gmail.com <mailto:chtom...@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-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
