I have code as follows :
String s = "";
for (My my : myList) {
s += my.getX();
}
Findbugs always reports error when I do this.
I have code as follows :
String s = "";
for (My my : myList) {
s += my.getX();
}
Findbugs always reports error when I do this.
The String object is immutable in Java. Each + means another object. You could use StringBuffer to minimize the amount of created objects.
I would use +
if you are manually concatenating,
String word = "Hello";
word += " World!";
However, if you are iterating and concatenating I would suggest StringBuilder
,
StringBuilder sb = new StringBuilder();
for (My my : myList) {
sb.append(my.getX());
}
The compiler can optimize some thing such as
"foo"+"bar"
To
StringBuilder s1=new StringBuilder(); s1.append("foo").append("bar");
However this is still suboptimal since it starts with a default size of 16. As with many things though you should find your biggest bottle necks and work your way down the list. It doesn't hurt to be in the habbit of using a SB pattern from the get go though, especially if you're able to calculate an optimal initialization size.
Premature optimization can be bad as well as it often reduces readability and is usually completely unnecessary. Use +
if it is more readable unless you actually have an overriding concern.
It is not 'always bad' to use "+". Using StringBuffer everywhere can make code really bulky.
If someone put a lot of "+" in the middle of an intensive, time-critical loop, I'd be annoyed. If someone put a lot of "+" in a rarely-used piece of code I would not care.
I would say use plus in the following:
String c = "a" + "b"
And use StringBuilder class everywhere else. As already mentioned in the first case it will be optimized by the compiler and it's more readable.
Each time you do string+=string
, it calls method like this:
private String(String s1, String s2) {
if (s1 == null) {
s1 = "null";
}
if (s2 == null) {
s2 = "null";
}
count = s1.count + s2.count;
value = new char[count];
offset = 0;
System.arraycopy(s1.value, s1.offset, value, 0, s1.count);
System.arraycopy(s2.value, s2.offset, value, s1.count, s2.count);
}
In case of StringBuilder, it comes to:
final void append0(String string) {
if (string == null) {
appendNull();
return;
}
int adding = string.length();
int newSize = count + adding;
if (newSize > value.length) {
enlargeBuffer(newSize);
}
string.getChars(0, adding, value, count);
count = newSize;
}
As you can clearly conclude, string + string
creates a lot of overhead, and in my opinion should be avoided if possible. If you think using StringBuilder is bulky or to long you can just make a method and use it indirectly, like:
public static String scat(String... vargs) {
StringBuilder sb = new StringBuilder();
for (String str : vargs)
sb.append(str);
return sb.toString();
}
And use it like:
String abcd = scat("a","b","c","d");
In C# I'm told its about as same as string.Concat();
. In your case it would be wise to write overload for scat, like:
public static String scat(Collection<?> vargs) {
StringBuilder sb = new StringBuilder();
for (Object str : vargs)
sb.append(str);
return sb.toString();
}
Then you can call it with:
result = scat(myList)
One of the reasons why FindBugs should argue about using concatenation operator (be it "+" or "+=") is localizability. In the example you gave it is not so apparent, but in case of the following code it is:
String result = "Scanning found " + Integer.toString(numberOfViruses) + " viruses";
If this looks somewhat familiar, you need to change your coding style. The problem is, it will sound great in English, but it could be a nightmare for translators. That's just because you cannot guarantee that order of the sentence will still be the same after translation – some languages will be translated to "1 blah blah", some to "blah blah 3". In such cases you should always use MessageFormat.format() to build compound sentences and using concatenation operator is clearly internationalization bug.
BTW. I put another i18n defect here, could you spot it?