RFR: 8264029: Replace uses of StringBuffer with StringBuilder in java.base
Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'` As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.base` Similar cleanup in the past - https://bugs.openjdk.java.net/browse/JDK-8041679 ------------- Commit messages: - [PATCH] Replaces usages of StringBuffer with StringBuilder where appropriate in java.base Changes: https://git.openjdk.java.net/jdk/pull/2922/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2922&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264029 Stats: 12 lines in 3 files changed: 0 ins; 3 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/2922.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2922/head:pull/2922 PR: https://git.openjdk.java.net/jdk/pull/2922
On Wed, 10 Mar 2021 19:47:00 GMT, Andrey Turbanov <github.com+741251+turbanoff@openjdk.org> wrote:
Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'` As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.base` Similar cleanup in the past - https://bugs.openjdk.java.net/browse/JDK-8041679
This looks good to me. I have created the bug for it: https://bugs.openjdk.java.net/browse/JDK-8264029 -- please change the PR synopsis to "8264029: Replace uses of StringBuffer with StringBuilder in java.base" ------------- Marked as reviewed by shade (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2922
On Wed, 10 Mar 2021 19:47:00 GMT, Andrey Turbanov <github.com+741251+turbanoff@openjdk.org> wrote:
Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'` As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.base` Similar cleanup in the past - https://bugs.openjdk.java.net/browse/JDK-8041679
src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1954:
1952: for (int i = 0; i < 4; ++i) { 1953: if (i > 0) { 1954: sb.append(" ");
Consider using `String.repeat` here and on L1960 for clarity. src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1954:
1952: for (int i = 0; i < 4; ++i) { 1953: if (i > 0) { 1954: sb.append(" ");
Consider using `String.repeat` here and on L1960 for clarity. ------------- PR: https://git.openjdk.java.net/jdk/pull/2922
On Tue, 23 Mar 2021 12:38:06 GMT, Pavel Rappo <prappo@openjdk.org> wrote:
Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'` As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.base` Similar cleanup in the past - https://bugs.openjdk.java.net/browse/JDK-8041679
src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1954:
1952: for (int i = 0; i < 4; ++i) { 1953: if (i > 0) { 1954: sb.append(" ");
Consider using `String.repeat` here and on L1960 for clarity.
I'm not sure how String.repeat can be used here. Repeated String is not constant and different for each iteration.
src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1954:
1952: for (int i = 0; i < 4; ++i) { 1953: if (i > 0) { 1954: sb.append(" ");
Consider using `String.repeat` here and on L1960 for clarity.
I don't think it can be used here. ------------- PR: https://git.openjdk.java.net/jdk/pull/2922
On Tue, 23 Mar 2021 20:44:17 GMT, Andrey Turbanov <github.com+741251+turbanoff@openjdk.org> wrote:
src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java line 1954:
1952: for (int i = 0; i < 4; ++i) { 1953: if (i > 0) { 1954: sb.append(" ");
Consider using `String.repeat` here and on L1960 for clarity.
I'm not sure how String.repeat can be used here. Repeated String is not constant and different for each iteration.
Long runs of whitespace, especially in blank strings, may have poor readability. I was thinking that " ".repeat(7) and " ".repeat(10) might read better than " " and " " respectively. If you don't agree, then simply disregard what I said. If you agree but worry about negative performance implications, consider creating these strings before the respective for-loops. ------------- PR: https://git.openjdk.java.net/jdk/pull/2922
On Tue, 23 Mar 2021 21:54:33 GMT, Pavel Rappo <prappo@openjdk.org> wrote:
I'm not sure how String.repeat can be used here. Repeated String is not constant and different for each iteration.
Long runs of whitespace, especially in blank strings, may have poor readability. I was thinking that
" ".repeat(7) and " ".repeat(10)
might read better than
" " and " "
respectively. If you don't agree, then simply disregard what I said. If you agree but worry about negative performance implications, consider creating these strings before the respective for-loops.
I suggest we ignore the use case for `String.repeat` here, and leave the patch as is. ------------- PR: https://git.openjdk.java.net/jdk/pull/2922
On Wed, 10 Mar 2021 19:47:00 GMT, Andrey Turbanov <github.com+741251+turbanoff@openjdk.org> wrote:
Found by IntelliJ IDEA inspection `Java | Java language level migration aids | Java 5 | 'StringBuffer' may be 'StringBuilder'` As suggested in https://github.com/openjdk/jdk/pull/1507#issuecomment-757369003 I've created separate PR for module `java.base` Similar cleanup in the past - https://bugs.openjdk.java.net/browse/JDK-8041679
This pull request has now been integrated. Changeset: fbbd98ba Author: Andrey Turbanov <turbanoff@gmail.com> Committer: Aleksey Shipilev <shade@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/fbbd98ba Stats: 12 lines in 3 files changed: 0 ins; 3 del; 9 mod 8264029: Replace uses of StringBuffer with StringBuilder in java.base Reviewed-by: shade ------------- PR: https://git.openjdk.java.net/jdk/pull/2922
participants (3)
-
Aleksey Shipilev
-
Andrey Turbanov
-
Pavel Rappo