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