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
