Re: [9] RFR 8055251: Re-examine Integer.parseInt and Long.parseLong methods
On 09/10/2014 01:55 PM, Claes Redestad wrote: On 09/05/2014 03:49 PM, Alan Bateman wrote: On 05/09/2014 14:03, Claes Redestad wrote: Hi, I'm requesting reviews and a sponsor for these changes to the recently added parse methods (8041972), suggested during discussions on net-dev: bug: https://bugs.openjdk.java.net/browse/JDK-8055251 webrev: http://cr.openjdk.java.net/~redestad/8055251/webrev.1/ Thanks for doing this, I think the API is much better now and much less error prone. The drive-by fix to the index in the NumberFormatException also looks good. Thanks! /Claes Looks good to me, nitpicking, line 672 of Long.java, I think it's better to use a for instead of a while, the incrementation will be more obvious. Also, but it's not fully related to this patch, the first line of parseInt/parseLong(CharSequence, int, int, int) is uncommon, s = Objects.requireNonNull(s); can be simplified to Objects.requireNonNull(s); regards, Rémi -Alan.
Re: [9] RFR 8055251: Re-examine Integer.parseInt and Long.parseLong methods
On 09/05/2014 03:49 PM, Alan Bateman wrote: On 05/09/2014 14:03, Claes Redestad wrote: Hi, I'm requesting reviews and a sponsor for these changes to the recently added parse methods (8041972), suggested during discussions on net-dev: bug: https://bugs.openjdk.java.net/browse/JDK-8055251 webrev: http://cr.openjdk.java.net/~redestad/8055251/webrev.1/ Thanks for doing this, I think the API is much better now and much less error prone. The drive-by fix to the index in the NumberFormatException also looks good. Thanks! /Claes -Alan.
Re: [9] RFR 8055251: Re-examine Integer.parseInt and Long.parseLong methods
On 05/09/2014 14:03, Claes Redestad wrote: Hi, I'm requesting reviews and a sponsor for these changes to the recently added parse methods (8041972), suggested during discussions on net-dev: bug: https://bugs.openjdk.java.net/browse/JDK-8055251 webrev: http://cr.openjdk.java.net/~redestad/8055251/webrev.1/ Thanks for doing this, I think the API is much better now and much less error prone. The drive-by fix to the index in the NumberFormatException also looks good. -Alan.
[9] RFR 8055251: Re-examine Integer.parseInt and Long.parseLong methods
Hi, I'm requesting reviews and a sponsor for these changes to the recently added parse methods (8041972), suggested during discussions on net-dev: bug: https://bugs.openjdk.java.net/browse/JDK-8055251 webrev: http://cr.openjdk.java.net/~redestad/8055251/webrev.1/ discussion:http://mail.openjdk.java.net/pipermail/net-dev/2014-August/008625.html Changes: - Removethe following methods: Integer::parseInt(CharSequence s, int radix, int beginIndex) Integer::parseUnsignedInt(CharSequence s, int radix, int beginIndex) Long::parseLong(CharSequence s, int radix, int beginIndex) Long::parseUnsignedLong(CharSequence s, int radix, int beginIndex) - Move radix parameter to the end in the following methods: Integer::parseInt(CharSequence s, int radix, int beginIndex, int endIndex) Integer::parseUnsignedInt(CharSequence s, int radix, int beginIndex, int endIndex) Long::parseLong(CharSequence s, int radix, int beginIndex, int endIndex) Long::parseUnsignedLong(CharSequence s, int radix, int beginIndex, int endIndex) - Change all places in the code where the methods already have been adopted - Add some tests for parseUnsignedInt/-Long - Ensure that when parsing fails the correct index is reported in the exception /Claes
Re: Integer.parseInt
+1 to Benedict and Martin -Ulf Am 12.04.2012 11:35, schrieb Martin Desruisseaux: Le 12/04/12 10:57, Benedict Elliott Smith a écrit : I think in this case it is reasonable to leave it to the user to ensure that the input remains consistent for the duration of the call. It can be documented if necessary, but as I say I think all imperative methods come with that caveat by definition. Calling the toString() method as a solution is really no better than asking the user to do the same, albeit a little neater; the only reason a CharSequence method would be preferred is that you can avoid unnecessary object allocation. For a very lightweight method like parseInt, this can dramatically reduce the impact of making the call. We faced this issue in our project when parsing OpenStreetMap data (an open source alternative to Google Map). The amount of integers to parse is so large that the calls to the 'toString()' method has been identified as a significant bottleneck by the NetBeans profiler. We tried a copy of the 'parseInt' method with the String argument replaced by CharSequence, and noticed a significant performance gain. Martin
Re: Integer.parseInt
Le 12/04/12 10:57, Benedict Elliott Smith a écrit : I think in this case it is reasonable to leave it to the user to ensure that the input remains consistent for the duration of the call. It can be documented if necessary, but as I say I think all imperative methods come with that caveat by definition. Calling the toString() method as a solution is really no better than asking the user to do the same, albeit a little neater; the only reason a CharSequence method would be preferred is that you can avoid unnecessary object allocation. For a very lightweight method like parseInt, this can dramatically reduce the impact of making the call. We faced this issue in our project when parsing OpenStreetMap data (an open source alternative to Google Map). The amount of integers to parse is so large that the calls to the 'toString()' method has been identified as a significant bottleneck by the NetBeans profiler. We tried a copy of the 'parseInt' method with the String argument replaced by CharSequence, and noticed a significant performance gain. Martin
Re: Integer.parseInt
> > Remi and I have in the past had differences of opinion on the utility of > introducing CharSequence versions of such methods. > > One benefit to using a string is that the object is immutable; there are > no time-of-check-versus-time-of-**use conditions to worry about. Robust > code should arguably work sensibly even with mutable CharSequences, and the > easiest way to ensure that is to call the toString method of a CharSequence > passed as a parameter. > > -Joe > It isn't typical to require that arguments to a method be immutable, and it is generally implicit that any arguments to a method that are mutated during the method's execution result in undefined output (to perhaps varying degrees). In an imperative object oriented language, attempting to eliminate any possibility of argument mutability is surely a doomed endeavour. I think in this case it is reasonable to leave it to the user to ensure that the input remains consistent for the duration of the call. It can be documented if necessary, but as I say I think all imperative methods come with that caveat by definition. Calling the toString() method as a solution is really no better than asking the user to do the same, albeit a little neater; the only reason a CharSequence method would be preferred is that you can avoid unnecessary object allocation. For a very lightweight method like parseInt, this can dramatically reduce the impact of making the call.
Re: Integer.parseInt
On 4/11/2012 7:45 AM, Rémi Forax wrote: On 04/11/2012 04:18 PM, Benedict Elliott Smith wrote: Hi, Looking at the source code, it doesn't appear as though there is any reason to require the input be a String - only length() and charAt() are called, which are both declared by CharSequence. Is there a reason I can't fathom, or was it an oversight? It would be nice to widen the type for this (and potentially other equivalent methods). Integer.parseInt (1.0 or 1.1) pre-date CharSequence (1.4), that's why it use a String an not a CharSequence. If you don't want to break all already compiled programs, you can't just replace String by CharSequence because the exact signature of the method (with the parameter types) is encoded in the bytecode. Joe Darcy write a cool blog post on that [1]. That is a kinder description of the blog post than I would expect :-) FYI, a fuller exploration of that issue in a broader context is written up in: http://cr.openjdk.java.net/~darcy/OpenJdkDevGuide/OpenJdkDevelopersGuide.v0.777.html The best here is to add new methods that takes a CharSequence, move the code that use a String in them and change the method that takes a String to delegate to the one that use a CharSequence. cheers, Rémi [1] https://blogs.oracle.com/darcy/entry/kinds_of_compatibility Remi and I have in the past had differences of opinion on the utility of introducing CharSequence versions of such methods. One benefit to using a string is that the object is immutable; there are no time-of-check-versus-time-of-use conditions to worry about. Robust code should arguably work sensibly even with mutable CharSequences, and the easiest way to ensure that is to call the toString method of a CharSequence passed as a parameter. -Joe
Re: Integer.parseInt
Sounds like a perfectly good reason! - and also that it should be a relatively safe change to implement. Any volunteers? I'd be happy to, but I expect the overhead for having a non-contributor do it would exceed the actual work by several orders of magnitude. On 11 April 2012 15:45, Rémi Forax wrote: > On 04/11/2012 04:18 PM, Benedict Elliott Smith wrote: > >> Hi, >> >> Looking at the source code, it doesn't appear as though there is any >> reason >> to require the input be a String - only length() and charAt() are called, >> which are both declared by CharSequence. Is there a reason I can't fathom, >> or was it an oversight? It would be nice to widen the type for this (and >> potentially other equivalent methods). >> > > Integer.parseInt (1.0 or 1.1) pre-date CharSequence (1.4), > that's why it use a String an not a CharSequence. > > If you don't want to break all already compiled programs, > you can't just replace String by CharSequence because the exact signature > of the method > (with the parameter types) is encoded in the bytecode. > Joe Darcy write a cool blog post on that [1]. > > The best here is to add new methods that takes a CharSequence, move the > code that use > a String in them and change the method that takes a String to delegate to > the one that use > a CharSequence. > > cheers, > Rémi > [1] > https://blogs.oracle.com/**darcy/entry/kinds_of_**compatibility<https://blogs.oracle.com/darcy/entry/kinds_of_compatibility> > > >
Re: Integer.parseInt
On 04/11/2012 04:18 PM, Benedict Elliott Smith wrote: Hi, Looking at the source code, it doesn't appear as though there is any reason to require the input be a String - only length() and charAt() are called, which are both declared by CharSequence. Is there a reason I can't fathom, or was it an oversight? It would be nice to widen the type for this (and potentially other equivalent methods). Integer.parseInt (1.0 or 1.1) pre-date CharSequence (1.4), that's why it use a String an not a CharSequence. If you don't want to break all already compiled programs, you can't just replace String by CharSequence because the exact signature of the method (with the parameter types) is encoded in the bytecode. Joe Darcy write a cool blog post on that [1]. The best here is to add new methods that takes a CharSequence, move the code that use a String in them and change the method that takes a String to delegate to the one that use a CharSequence. cheers, Rémi [1] https://blogs.oracle.com/darcy/entry/kinds_of_compatibility
Integer.parseInt
Hi, Looking at the source code, it doesn't appear as though there is any reason to require the input be a String - only length() and charAt() are called, which are both declared by CharSequence. Is there a reason I can't fathom, or was it an oversight? It would be nice to widen the type for this (and potentially other equivalent methods).
