Warning in StringBuffer and StringBuilder
Asked Answered
R

2

25

I have a StringBuffer initialized outside for loop and inside for loop I am concatenating some strings.

I am getting the warning

'StringBuffer stringBuffer' may be declared as 'StringBuilder'

and

string concatenation as argument to 'stringbuilder.append()' call

Then I changed that StringBuffer to StringBuilder, since it is comparatively faster than StringBuffer. Now I am getting the warning as

string concatenation as argument to 'stringbuilder.append()' call

Sample code:

public static String stringConcat(String[] words) {
    StringBuffer stringBuffer = new StringBuffer();
    for (String word : words) {
        stringBuffer.append(word).append(" ");
    }
    return stringBuffer.toString();
}

Why I am getting these warnings.

Edit Actual code:

stringBuffer.append(word.substring(0, 1).toUpperCase() + word.substring(1).toLowerCase()).append(" ");
Reason answered 30/3, 2017 at 3:36 Comment(3)
You should definitely prefer StringBuilder to StringBuffer, and if you can use Java 8+ language features - I would prefer return Stream.of(words).collect(Collectors.joining(" "));Coherent
It's exactly what the message says. Don't concatenate strings inside append(). Also don't post sample code that obfuscates the problem.Paranoid
Okay. Thank you @ElliottFrisch. But still I am getting the warning as string concatenation as argument to 'stringbuilder.append()' call in stringBuffer.append(word).append(" ");. I can't use java 8 features in my application.Reason
H
38

The point is : you are still using the + operator for strings in your expression that you give to append():

... word.substring(0, 1).toUpperCase() + word...

That negates the whole point of using a StringBuilder (or StringBuffer).

Instead: simply call append() twice! The core idea of using a buffer/builder is to concat your desired result by only using append calls; like:

append(word.substring(0, 1).toUpperCase()).append(word...
Hierarch answered 30/3, 2017 at 3:54 Comment(3)
I do not see that "this negates the whole point of using a StringBuilder". And sometimes it is much more readable to append substrings that where put together with +. One might consider using a WarningSupression in IntelliJ or the like such as //noinspection StringConcatenationInsideStringBufferAppendGrenada
@Grenada The point of a StringBuilder is efficiency. Remember that the compiler will turn the String+ into ... another StringBuilder instance, that will be used to concat all the strings that you are + together. So instead to directly appending to your own manually defined builder, another builder is created, stuff gets appended, toString() gets called ... for no reason whatsoever. Sure, there are many ways to do this, like String.format() and whatnot. But combining builder and string+ ... it is just a very surprising solution.Hierarch
And a key attribute of clean code: it doesn't surprise its readers!Hierarch
L
6

Use StringBuilder instead of StringBuffer

 public static String stringConcat(String[] words) {
    StringBuilder stringBuilder = new StringBuilder();

    for (String word : words) {
        stringBuilder.append(word).append(" ");
    }
    return stringBuilder.toString();
  }
Lichtenfeld answered 30/3, 2017 at 3:40 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.