Re: [lang] LANG-1247: FastDatePrinter generates Date objects wastefully closes #168

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [lang] LANG-1247: FastDatePrinter generates Date objects wastefully closes #168

Kristian Rosenvold
2016-07-05 22:44 GMT+02:00  <[hidden email]>:
> LANG-1247: FastDatePrinter generates Date objects wastefully
> closes #168
>      public StringBuffer format(final long millis, final StringBuffer buf) {
> -        return format(new Date(millis), buf);
> +        final Calendar c = newCalendar();
> +        c.setTimeInMillis(millis);
> +        return applyRules(c, buf);


Arguably I might be a few years behind on developments regarding
java.lang.Calendar within the JDK, but from previous experience the
Calendar class is about as inefficient as things get, and anything
touching the calendar sprays your heap with objects. As far as I can
see, this just seems to be swapping around object allocation, probably
to the worse (instead of one visible object allocation within
commons-lang you're creating a fair bit of com.sun allocations as far
as I know)

I'd really like to see som evidence that this is an improvement, if
nothing else to enlighten me :)

Kristian

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [lang] LANG-1247: FastDatePrinter generates Date objects wastefully closes #168

sebb-2-2
On 6 July 2016 at 07:19, Kristian Rosenvold <[hidden email]> wrote:
> 2016-07-05 22:44 GMT+02:00  <[hidden email]>:
>> LANG-1247: FastDatePrinter generates Date objects wastefully
>> closes #168
>>      public StringBuffer format(final long millis, final StringBuffer buf) {
>> -        return format(new Date(millis), buf);

Have a look at the format method.

It uses Calendar much as below.

>> +        final Calendar c = newCalendar();
>> +        c.setTimeInMillis(millis);
>> +        return applyRules(c, buf);
>
>
> Arguably I might be a few years behind on developments regarding
> java.lang.Calendar within the JDK, but from previous experience the
> Calendar class is about as inefficient as things get, and anything
> touching the calendar sprays your heap with objects. As far as I can
> see, this just seems to be swapping around object allocation, probably
> to the worse (instead of one visible object allocation within
> commons-lang you're creating a fair bit of com.sun allocations as far
> as I know)
>
> I'd really like to see som evidence that this is an improvement, if
> nothing else to enlighten me :)

See above and the JIRA:

in this case the Date is *only* used to transport the long time to the
Calendar instance.

If you wish to eliminate Calendar in this case, that's another matter.

> Kristian
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [lang] LANG-1247: FastDatePrinter generates Date objects wastefully closes #168

Kristian Rosenvold
2016-07-06 10:12 GMT+02:00 sebb <[hidden email]>:
> Have a look at the format method.
>
> It uses Calendar much as below.
>
>>> +        final Calendar c = newCalendar();
>>> +        c.setTimeInMillis(millis);
>>> +        return applyRules(c, buf);


Yeah, I see now. Seems like I got lost in the overloads :)

Thanks,

Kristian

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins
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.

Cheers,
-Rob



Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Benedikt Ritter-4
Hello Rob,

Rob Tompkins <[hidden email]> 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.

Regards,
Benedikt


> Cheers,
> -Rob
>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins
Hi Benedikt,

Thanks for the insights here.

> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email]> wrote:
>
> Hello Rob,
>
> Rob Tompkins <[hidden email] <mailto:[hidden email]>> 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

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

garydgregory
Can this all be solved with better Javadocs?

Gary

On Jul 29, 2016 7:59 AM, "Rob Tompkins" <[hidden email]> wrote:

> Hi Benedikt,
>
> Thanks for the insights here.
>
> > On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email]> wrote:
> >
> > Hello Rob,
> >
> > Rob Tompkins <[hidden email] <mailto:[hidden email]>> 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
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins

> On Jul 30, 2016, at 12:41 PM, Gary Gregory <[hidden email]> 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" <[hidden email]> wrote:
>
>> Hi Benedikt,
>>
>> Thanks for the insights here.
>>
>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email]> wrote:
>>>
>>> Hello Rob,
>>>
>>> Rob Tompkins <[hidden email] <mailto:[hidden email]>> 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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Benedikt Ritter-4
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 <[hidden email]> schrieb am Sa., 30. Juli 2016 um 21:17:

>
> > On Jul 30, 2016, at 12:41 PM, Gary Gregory <[hidden email]>
> 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" <[hidden email]> wrote:
> >
> >> Hi Benedikt,
> >>
> >> Thanks for the insights here.
> >>
> >>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email]>
> wrote:
> >>>
> >>> Hello Rob,
> >>>
> >>> Rob Tompkins <[hidden email] <mailto:[hidden email]>> 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: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins


> On Jul 31, 2016, at 6:23 AM, Benedikt Ritter <[hidden email]> 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 <[hidden email]> schrieb am Sa., 30. Juli 2016 um 21:17:
>
>>
>>>> On Jul 30, 2016, at 12:41 PM, Gary Gregory <[hidden email]>
>>> 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" <[hidden email]> wrote:
>>>>
>>>> Hi Benedikt,
>>>>
>>>> Thanks for the insights here.
>>>>
>>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email]>
>> wrote:
>>>>>
>>>>> Hello Rob,
>>>>>
>>>>> Rob Tompkins <[hidden email] <mailto:[hidden email]>> 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: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins
In reply to this post by Benedikt Ritter-4
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
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 <[hidden email]> 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 <[hidden email]> schrieb am Sa., 30. Juli 2016 um 21:17:
>
>>
>>> On Jul 30, 2016, at 12:41 PM, Gary Gregory <[hidden email]>
>> 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" <[hidden email]> wrote:
>>>
>>>> Hi Benedikt,
>>>>
>>>> Thanks for the insights here.
>>>>
>>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email]>
>> wrote:
>>>>>
>>>>> Hello Rob,
>>>>>
>>>>> Rob Tompkins <[hidden email] <mailto:[hidden email]>> 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: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins

> On Jul 31, 2016, at 3:03 PM, Rob Tompkins <[hidden email]> 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 <[hidden email] <mailto:[hidden email]>> 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 <[hidden email] <mailto:[hidden email]>> schrieb am Sa., 30. Juli 2016 um 21:17:
>>
>>>
>>>> On Jul 30, 2016, at 12:41 PM, Gary Gregory <[hidden email] <mailto:[hidden email]>>
>>> 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" <[hidden email] <mailto:[hidden email]>> wrote:
>>>>
>>>>> Hi Benedikt,
>>>>>
>>>>> Thanks for the insights here.
>>>>>
>>>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email] <mailto:[hidden email]>>
>>> wrote:
>>>>>>
>>>>>> Hello Rob,
>>>>>>
>>>>>> Rob Tompkins <[hidden email] <mailto:[hidden email]> <mailto:[hidden email] <mailto:[hidden email]>>> 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: [hidden email] <mailto:[hidden email]>
>>> For additional commands, e-mail: [hidden email] <mailto:[hidden email]>
>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Benedikt Ritter-4
Hello Rob,

I'm back from vacation and I will have a look at this issue again later
this week.

Benedikt

Rob Tompkins <[hidden email]> schrieb am Do., 11. Aug. 2016 um
16:48 Uhr:

>
> > On Jul 31, 2016, at 3:03 PM, Rob Tompkins <[hidden email]> 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 <[hidden email]
> <mailto:[hidden email]>> 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 <[hidden email] <mailto:[hidden email]>> schrieb
> am Sa., 30. Juli 2016 um 21:17:
> >>
> >>>
> >>>> On Jul 30, 2016, at 12:41 PM, Gary Gregory <[hidden email]
> <mailto:[hidden email]>>
> >>> 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" <[hidden email] <mailto:
> [hidden email]>> wrote:
> >>>>
> >>>>> Hi Benedikt,
> >>>>>
> >>>>> Thanks for the insights here.
> >>>>>
> >>>>>> On Jul 29, 2016, at 3:53 AM, Benedikt Ritter <[hidden email]
> <mailto:[hidden email]>>
> >>> wrote:
> >>>>>>
> >>>>>> Hello Rob,
> >>>>>>
> >>>>>> Rob Tompkins <[hidden email] <mailto:[hidden email]>
> <mailto:[hidden email] <mailto:[hidden email]>>> 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: [hidden email] <mailto:
> [hidden email]>
> >>> For additional commands, e-mail: [hidden email] <mailto:
> [hidden email]>
> >>>
> >>>
> >
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

jochen-2
On Tue, Aug 16, 2016 at 9:10 PM, Benedikt Ritter <[hidden email]> wrote:

>> > On Jul 31, 2016, at 3:03 PM, Rob Tompkins <[hidden email]> wrote:

>> > 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 suggest to consider the following:

- Replace isParsable(String) with (or, add a new method)
getParseableType(String), where the latter would return something like
     "UNPARSEABLE", "FLOAT", "DOUBLE", indicating the expected result
type for createNumber(String).

Jochen

--
The next time you hear: "Don't reinvent the wheel!"

http://www.keystonedevelopment.co.uk/wp-content/uploads/2014/10/evolution-of-the-wheel-300x85.jpg

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins

> On Aug 18, 2016, at 3:10 AM, Jochen Wiedmann <[hidden email]> wrote:
>
> On Tue, Aug 16, 2016 at 9:10 PM, Benedikt Ritter <[hidden email]> wrote:
>
>>>> On Jul 31, 2016, at 3:03 PM, Rob Tompkins <[hidden email]> wrote:
>
>>>> 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 suggest to consider the following:
>
> - Replace isParsable(String) with (or, add a new method)
> getParseableType(String), where the latter would return something like
>     "UNPARSEABLE", "FLOAT", "DOUBLE", indicating the expected result
> type for createNumber(String).

Maybe something more along the lines of isParsable(String), returning true if the string is parsable to any subclasses of java.lang.Number, and then as our other method we would have something along the lines of isParsable(String,class) returning true if we can parse to that specific subclass of java.lang.Number.

Either way, should this work go under LANG-1252 or a different issue?

-Rob

>
> Jochen
>
> --
> The next time you hear: "Don't reinvent the wheel!"
>
> http://www.keystonedevelopment.co.uk/wp-content/uploads/2014/10/evolution-of-the-wheel-300x85.jpg
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Rob Tompkins
I spent considerable time over the weekend here playing around with different ideas on how to accomplish the reconciliation of the methods at hand here in NumberUtils. The more I fiddle around with the issue, the more I really want to rename “isNumber,” “isParsable” and take “isNumber” (what would be “isJavaNumberLiteral”) and completely rewrite it.

I keep returning to this portion of comments:
https://github.com/apache/commons-lang/blob/master/src/test/java/org/apache/commons/lang3/math/NumberUtilsTest.java#L1218-L1223 <https://github.com/apache/commons-lang/blob/master/src/test/java/org/apache/commons/lang3/math/NumberUtilsTest.java#L1218-L1223>
/**
 * Tests isNumber(String) and tests that createNumber(String) returns
 * a valid number iff isNumber(String) returns false.
 */
@Test
public void testIsNumber() {


Further, to me, it feels like that isJavaNumberLiteral is not so far away from isParsable in terms of functionality. Clearly there would be some subtle differences, but they feel remarkably more similar than “isNumber” and “isParsable” are currently in the code base (with “isParsable” being substantially more simplistic).

The reason for another email to the M.L. here is: before getting too far into implementation, I wanted to bounce some of these ideas around again.

Implementation thoughts:
(1) Switch implementation “isNumber” -> “isParsable”
(2) Deprecate “isNumber” and have it call “isParsable"
(3) Rename “createNumber” to “parseNumber” (with Deprecation)
(4) Fix subtle differences between “parseNumber” and newly implemented “isParsable"
(5) Write “isJavaNumberLiteral” from beginning.

What do you guys think. Also, I don’t think I’d be comfortable making such extensive changes without some one like Benedikt putting another set of eyes on the work.

Cheers,
-Rob


> On Aug 19, 2016, at 4:53 PM, Rob Tompkins <[hidden email]> wrote:
>
>
>> On Aug 18, 2016, at 3:10 AM, Jochen Wiedmann <[hidden email]> wrote:
>>
>> On Tue, Aug 16, 2016 at 9:10 PM, Benedikt Ritter <[hidden email]> wrote:
>>
>>>>> On Jul 31, 2016, at 3:03 PM, Rob Tompkins <[hidden email]> wrote:
>>
>>>>> 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 suggest to consider the following:
>>
>> - Replace isParsable(String) with (or, add a new method)
>> getParseableType(String), where the latter would return something like
>>    "UNPARSEABLE", "FLOAT", "DOUBLE", indicating the expected result
>> type for createNumber(String).
>
> Maybe something more along the lines of isParsable(String), returning true if the string is parsable to any subclasses of java.lang.Number, and then as our other method we would have something along the lines of isParsable(String,class) returning true if we can parse to that specific subclass of java.lang.Number.
>
> Either way, should this work go under LANG-1252 or a different issue?
>
> -Rob
>
>>
>> Jochen
>>
>> --
>> The next time you hear: "Don't reinvent the wheel!"
>>
>> http://www.keystonedevelopment.co.uk/wp-content/uploads/2014/10/evolution-of-the-wheel-300x85.jpg
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: [LANG] LANG-1252 - NumberUtils.isNumber and NumberUtils.createNumber resolve inconsistently

Eric Barnhill
> Implementation thoughts:
> (1) Switch implementation “isNumber” -> “isParsable”
> (2) Deprecate “isNumber” and have it call “isParsable"
> (3) Rename “createNumber” to “parseNumber” (with Deprecation)
> (4) Fix subtle differences between “parseNumber” and newly implemented
> “isParsable"
> (5) Write “isJavaNumberLiteral” from beginning.
>


I took a look at the code. As far as design goes I think they are all good
improvements. I did notice this in the doc:

--
<p>Parsable numbers include those Strings understood by {@link
Integer#parseInt(String)},
     * {@link Long#parseLong(String)}, {@link Float#parseFloat(String)} or
     * {@link Double#parseDouble(String)}. This method can be used instead
of catching {@link java.text.ParseException}
     * when calling one of those methods.</p>
<p>Hexadecimal and scientific notations are <strong>not</strong> considered
parsable.
--

So this makes me wonder if a method called isParsable should in fact be
able to parse these numbers? Is this doc canonical somehow, or just how
previous contributors chose to think about the issue? In which case the doc
could be restated, saying for example "Hexadecimal and scientific notations
are <strong>not</strong> considered Number Literals."

Regarding point 5, this page

https://en.wikibooks.org/wiki/Java_Programming/Literals

uses the term "Numeric Literal", perhaps that is better phrasing than
"Number Literal"?

Eric