RFR [9] 8041972: Add improved parse/format methods for Long/Integer

[Claes Redestad]

On 2014-06-16 21:50, roger riggs wrote:
> Hi Claes,
>
> The descriptions of the new methods should take the same form as
> the coresponding existing methods.  The rationalization about 
> intermediary
> objects is not useful in describing the behavior of the method and 
> should be omitted.
Point taken!
>
> /**
>  * Parses the string argument starting at fromIndex as a signed 
> integer in the radix
>  * specified by the second argument.
> ...
>
>     static int parseInt(CharSequence s, int radix, int fromIndex)
>
> The terminology used in java.lang.String for offsets and indexes into 
> strings would
> be provide a consistent base for talking about substrings.
If we're taking cues from String.substring, I guess int beginIndex[, int 
fromIndex] would be more appropriate. How about:

     /**
      * Parses the character sequence argument in the specified {@code 
radix},
      * beginning at the specified {@code beginIndex} and extending to the
      * character at index {@code endIndex - 1}.
      *
      * @see java.lang.Integer#parseInt(String, int)
      * @param      s   the {@code CharSequence} containing the integer
      *                  representation to be parsed
      * @param      radix   the radix to be used while parsing {@code s}.
      * @param      beginIndex   the beginning index, inclusive.
      * @param      endIndex     the ending index, exclusive.
      * @return     the integer represented by the subsequence in the
      *             specified radix.
      */
     static int parseInt(CharSequence s, int radix, int beginIndex, int 
endIndex)

?

Thanks!

/Claes
>
> Thanks, Roger
>
>
> For example,
>
>
> On 6/15/2014 6:22 PM, Claes Redestad wrote:
>> I've updated the patch to use CharSequence in favor of String for all 
>> new methods, as well as ensuring all new methods are package private 
>> (for now):
>>
>> http://cr.openjdk.java.net/~redestad/8041972/webrev.2/
>>
>> Reviews appreciated!
>>
>> /Claes
>>
>> On 2014-06-15 14:29, Claes Redestad wrote:
>>>
>>> On 2014-06-15 13:48, Remi Forax wrote:
>>>>
>>>> On 06/15/2014 12:36 AM, Claes Redestad wrote:
>>>>> Hi,
>>>>>
>>>>> please review this patch to add offset based variants of 
>>>>> Integer.parseInt/parseUnsignedInt, 
>>>>> Long.parseLong/parseUnsignedLong and expose them through 
>>>>> JavaLangAccess along with formatUnsignedInt/-Long. This is 
>>>>> proposed to enable a number of minor optimizations, starting with 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8006627
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~redestad/8041972/webrev.0/
>>>>>
>>>>> Thanks!
>>>>>
>>>>> /Claes
>>>>
>>>> In my opinion, and as suggested in the description of 8041972, 
>>>> these methods should be public.
>>> Ultimately, yes. The ulterior motive here is to be able to backport 
>>> these to 8u40 (maybe even 7) for internal JDK use, while updating to 
>>> public (and dropping the SharedSecrets) should be done in a later, 
>>> 9-only update. Adding public methods would require CCC approval, 
>>> more detailed javadocs and possibly be part of a JEP, so pushing a 
>>> back-portable internal patch as a first step seems reasonable.
>>>> Having written my share of scanners/parsers in Java, these methods 
>>>> are really helpful but
>>>> given that they only rely on charAt/length of String, I think they 
>>>> should take a CharSequence and not a String as first parameter,
>>>> in order to support other storages of characters like by example a 
>>>> java.nio.CharBuffer.
>>>>
>>>> So for me, instead of adding a bunch of method in shared secrets, 
>>>> those method should be public and take a CharSequence.
>>> I wouldn't mind switching over to CharSequence for the newly added 
>>> methods. I wasn't around last time[1] this was proposed for the 
>>> existing methods, but I know there were arguments that changing to a 
>>> (possibly) mutable input type might have problematic side effects 
>>> that would form a valid argument against this switch, while I 
>>> personally share the opinion that it's up to the caller to enforce 
>>> immutability when necessary and that text parsing methods should all 
>>> use CharSequence when possible.
>>>>
>>>> Miinor nits, Integer.parseInt(String,int,int) don't need to test if 
>>>> s is null given it delegated to parseInt(String, int, int, int),
>>>> and Integer.parseInt(String) should directly call parseInt(s, 10, 
>>>> 0, s.length()) instead of calling parseInt(s, 10) that calls
>>>> parseInt(s, 10, 0) that calls parseInt(s, 10, 0, s.length()).
>>>>
>>>> cheers,
>>>> Rémi
>>> Not checking for null in parseInt(String, int, int) would mean we 
>>> would get a NPE calling s.length() in the call to parseInt(String, 
>>> int, int, int), so this is needed for compatibility. Microbenchmarks 
>>> suggests the extra check does not have any performance impact since 
>>> the JIT can easily prove that the inner null checks can be removed 
>>> when an outer method.
>>>
>>> Calling parseInt(s, 10, 0, s.length()) directly in place of 
>>> parseInt(s, 10, 0) would likewise require an earlier null check 
>>> (slightly more code duplication) while being performance neutral 
>>> either way.
>>>
>>> /Claes
>>>
>>> [1] 
>>> https://www.mail-archive.com/core-libs-dev@openjdk.java.net/msg09694.html
>>
>