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
