Can this all be solved with better Javadocs? 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 > >
