RFR: [8054714] Use StringJoiner where it makes the code cleaner
Hello everyone! This is a follower of the recently fixed [8051382] -- Optimize java.lang.reflect.Modifier.toString(). I found a dozen+ other places in jdk, where using the StringJoiner makes the code shorter and cleaner. Would you please help review the fix? http://cr.openjdk.java.net/~igerasim/8054714/0/webrev/ Sincerely yours, Ivan
Looks good to me, if that matters... Not sure if worth considering, but for simple cases like src/share/classes/java/util/PropertyPermission.java, you could return the possible constant values instead: static String getActions(int mask) { switch (mask & 0x3) { case 1: return SecurityConstants.READ; case 2: return SecurityConstants.WRITE; case 3: return SecurityConstants.RW; default: return ""; } } Also, now that it's backed by a String[], some of these cases could be optimized if StringJoiner had a way to set/ensure capacity (what leaky abstractions?). Thanks! /Claes On 2014-08-08 23:26, Ivan Gerasimov wrote:
Hello everyone!
This is a follower of the recently fixed [8051382] -- Optimize java.lang.reflect.Modifier.toString().
I found a dozen+ other places in jdk, where using the StringJoiner makes the code shorter and cleaner.
Would you please help review the fix?
http://cr.openjdk.java.net/~igerasim/8054714/0/webrev/
Sincerely yours, Ivan
On 09.08.2014 4:44, Claes Redestad wrote:
Looks good to me, if that matters...
Thanks Claes!
Not sure if worth considering, but for simple cases like src/share/classes/java/util/PropertyPermission.java, you could return the possible constant values instead:
static String getActions(int mask) { switch (mask & 0x3) { case 1: return SecurityConstants.READ; case 2: return SecurityConstants.WRITE; case 3: return SecurityConstants.RW; default: return ""; } }
Yes, this will surely work faster. I've incorporated your suggestion: http://cr.openjdk.java.net/~igerasim/8054714/1/webrev/
Also, now that it's backed by a String[], some of these cases could be optimized if StringJoiner had a way to set/ensure capacity (what leaky abstractions?).
I wasn't planning to change the StringJoiner's API with this fix :-) Sincerely yours, Ivan
Thanks!
/Claes
On 2014-08-08 23:26, Ivan Gerasimov wrote:
Hello everyone!
This is a follower of the recently fixed [8051382] -- Optimize java.lang.reflect.Modifier.toString().
I found a dozen+ other places in jdk, where using the StringJoiner makes the code shorter and cleaner.
Would you please help review the fix?
http://cr.openjdk.java.net/~igerasim/8054714/0/webrev/
Sincerely yours, Ivan
On Aug 9, 2014, at 8:32 AM, Ivan Gerasimov <ivan.gerasimov@oracle.com> wrote:
Yes, this will surely work faster. I've incorporated your suggestion: http://cr.openjdk.java.net/~igerasim/8054714/1/webrev/
Looks ok. V. minor point, rogue space: sun/net/www/protocol/http/HttpURLConnection.java 2014-08-09 10:34:38.656983033 +0400 + if (! cookie.isHttpOnly()) Paul.
Hello, In src/share/classes/java/util/PropertyPermission.java, I believe the separator in 327 static String getActions(int mask) { 328 StringJoiner sj = new StringJoiner(", "); should be "," (a single comma) rather than ", " (a comma followed by a space). In src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java, 1390 StringJoiner cookieValue = new StringJoiner(", "); the separator used in the original code seems to be "; " rather than ", ". Cheers, -Joe On 08/08/2014 02:26 PM, Ivan Gerasimov wrote:
Hello everyone!
This is a follower of the recently fixed [8051382] -- Optimize java.lang.reflect.Modifier.toString().
I found a dozen+ other places in jdk, where using the StringJoiner makes the code shorter and cleaner.
Would you please help review the fix?
http://cr.openjdk.java.net/~igerasim/8054714/0/webrev/
Sincerely yours, Ivan
Thanks Joe! I should have been more accurate with keeping the original delimiters. Please find the updated webrev here: http://cr.openjdk.java.net/~igerasim/8054714/1/webrev/ Sincerely yours, Ivan On 09.08.2014 5:38, Joe Darcy wrote:
Hello,
In src/share/classes/java/util/PropertyPermission.java, I believe the separator in
327 static String getActions(int mask) { 328 StringJoiner sj = new StringJoiner(", ");
should be "," (a single comma) rather than ", " (a comma followed by a space).
In src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java,
1390 StringJoiner cookieValue = new StringJoiner(", ");
the separator used in the original code seems to be "; " rather than ", ".
Cheers,
-Joe
On 08/08/2014 02:26 PM, Ivan Gerasimov wrote:
Hello everyone!
This is a follower of the recently fixed [8051382] -- Optimize java.lang.reflect.Modifier.toString().
I found a dozen+ other places in jdk, where using the StringJoiner makes the code shorter and cleaner.
Would you please help review the fix?
http://cr.openjdk.java.net/~igerasim/8054714/0/webrev/
Sincerely yours, Ivan
participants (4)
-
Claes Redestad
-
Ivan Gerasimov
-
Joe Darcy
-
Paul Sandoz