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(" ");
GhostCat
  • 137,827
  • 25
  • 176
  • 248
Abish R
  • 1,537
  • 3
  • 18
  • 36
  • 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(" "));` – Elliott Frisch Mar 30 '17 at 03:38
  • 4
    It's exactly what the message says. Don't concatenate strings inside `append()`. Also don't post sample code that obfuscates the problem. – shmosel Mar 30 '17 at 03:44
  • 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. – Abish R Mar 30 '17 at 03:45

2 Answers2

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...
GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • 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 StringConcatenationInsideStringBufferAppend – Simeon Aug 26 '21 at 15:57
  • 1
    @Simeon 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. – GhostCat Aug 26 '21 at 18:35
  • And a key attribute of clean code: it doesn't surprise its readers! – GhostCat Aug 26 '21 at 18:35
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();
  }
Komal12
  • 3,340
  • 4
  • 16
  • 25