Hello Rob, I'm back from vacation and I will have a look at this issue again later this week.
Benedikt Rob Tompkins <chtom...@gmail.com> schrieb am Do., 11. Aug. 2016 um 16:48 Uhr: > > > On Jul 31, 2016, at 3:03 PM, Rob Tompkins <chtom...@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 <brit...@apache.org > <mailto: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. > >> > >> Benedikt > >> > >> Rob Tompkins <chtom...@gmail.com <mailto: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 > <mailto: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 <mailto: > chtom...@gmail.com>> wrote: > >>>> > >>>>> Hi Benedikt, > >>>>> > >>>>> Thanks for the insights here. > >>>>> > >>>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <brit...@apache.org > <mailto:brit...@apache.org>> > >>> wrote: > >>>>>> > >>>>>> Hello Rob, > >>>>>> > >>>>>> Rob Tompkins <chtom...@gmail.com <mailto:chtom...@gmail.com> > <mailto: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 <mailto: > dev-unsubscr...@commons.apache.org> > >>> For additional commands, e-mail: dev-h...@commons.apache.org <mailto: > dev-h...@commons.apache.org> > >>> > >>> > > > >