Skip to content

Replace StringBuffer with StringBuilder in org.cactoos.text.Replaced (#1410) - #1891

Open
mvanhorn wants to merge 1 commit into
yegor256:masterfrom
mvanhorn:fix/1410-replaced-stringbuilder
Open

Replace StringBuffer with StringBuilder in org.cactoos.text.Replaced (#1410)#1891
mvanhorn wants to merge 1 commit into
yegor256:masterfrom
mvanhorn:fix/1410-replaced-stringbuilder

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Closes #1410.

Replaces StringBuffer with StringBuilder in org.cactoos.text.Replaced. The buffer is scoped to a single lambda invocation and never escapes that thread, so the synchronization StringBuffer inherits from AbstractStringBuilder is wasted work.

Matcher.appendReplacement(StringBuilder, String) and Matcher.appendTail(StringBuilder) have been available since Java 9; the project's pom.xml already targets Java 17, so the StringBuilder overloads compile cleanly.

The existing ReplacedTest covers the public behaviour and stays unchanged.

…egor256#1410)

Matcher.appendReplacement(StringBuilder, String) and
Matcher.appendTail(StringBuilder) have been available since Java 9.
The pom.xml targets Java 17, so the StringBuilder overloads are
available. StringBuilder skips the synchronization StringBuffer
inherits from AbstractStringBuilder, which is wasted work here
since the buffer is scoped to a single lambda invocation and never
escapes that thread.

@GHX5T-SOL GHX5T-SOL left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified current head 7fabfcf6054a2b49236c802526d0f40a3a4c2a76.

The change is scoped to the production Replaced buffer used by Matcher.appendReplacement / appendTail. Since pom.xml compiles with Java 17, the StringBuilder overloads are available, and the buffer is local to one mapped string operation. I also checked that this removes the only production StringBuffer occurrence; the remaining StringBuffer references are test coverage for APIs that accept it.

Local validation on this head:

  • mvn -q -DskipTests compile -> passed.
  • mvn -q -Dtest=ReplacedTest -Djacoco.skip=true -Dhone.skip=true test -> 11 tests, 0 failures, 0 errors, 0 skipped.
  • git grep -n "StringBuffer" origin/master -- src/main/java src/test/java vs HEAD -> production Replaced.java occurrence removed; only test uses remain.
  • git diff --check origin/master...HEAD -> clean.
  • git diff --no-ext-diff origin/master...HEAD | gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found.

Hosted Maven is currently unstable because the macOS job fails inside hone-maven-plugin with Cannot run program "docker", and the other Maven jobs are cancelled. I don't see that as caused by this one-line Replaced change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

org.cactoos.text.Replaced uses StringBuffer instead of StringBuilder

2 participants