using stringbuilder with for loop

Question

 private String generateSuffix(List<String[]> searches, List<String[]> sorts) {
        String ret = "";
        if (searches != null && !searches.isEmpty()) {
            ret += "where ";
            for (String[] search : searches) {
                if (search.length < 3) {
                    continue;
                }
                ret += search[0] + search[1] + search[2] + " and ";
            }
            ret = ret.substring(0, ret.length() - 5);
        }

        ret += " order by ";
        if (sorts != null && !sorts.isEmpty()) {
            for (String[] sort : sorts) {
                if (sort.length < 2) {
                    continue;
                }
                ret += sort[0] + " " + sort[1] + ",";
            }
        }
        return ret;
    }
}

My SONAR scan indicated that using '+' to concatenate strings in a loop is a major error. It does not give much insight as to how this error can be fixed. Can StringBuilder be used in this case? If so, how would I go about implementing the StringBuilder with the enhanced for loop? I'm not very familiar with using StringBuilder.

Also, can I add a StringBuilder within a list as shown below? I need to get rid of the code "value += ds;" and value = d + "" ".

private List<TransactionSearchField> checkSearches(List<TransactionSearchField> searchesIn) {
    List<TransactionSearchField> searches = new ArrayList<TransactionSearchField>();
    if (searchesIn != null) {
        for (TransactionSearchField search : searchesIn) {
            String value = search.getValue();
            if (value != null && value.length() > 0) {
                if (search.getDataType().equals(double.class)) {
                    String[] dSplit = value.split(",");
                    value = "";
                    for (String ds : dSplit) {
                        value += ds;
                    }
                    double d;
                    try {
                        d = Double.parseDouble(value);
                        value = d + "";
                    } catch (IllegalArgumentException e) {
                        value = "";
                    }
                } else if (search.getDataType().equals(Date.class)) {
                    value = formatDate(generateDate(value));
                }
                searches.add(new TransactionSearchField(search.getDataType(), search.getTableField(), search.getLogic(), value, search.getViewField()));
            }
        }
    }
    return searches;
}

StringBuilder sb = new StringBuilder(); for (String s : yourStringArray) { sb.append(s); } String result = sb.toString(); Copy it and format it — Jean-François Savard
– Jean-François Savard, Commented Oct 2, 2015 at 14:25
If you use java8 using StringJoiner may be an option. @Rainbolt: Not a duplicate of that question. The OP only removes the last " and ". Also it's not a question about the performance. — fabian
– fabian, Commented Oct 2, 2015 at 14:44
The reason string concatenation in loops is bad is because strings are immutable in Java. So every time you concatenate, you are creating an entirely new String. Of course, if your loop is small, the difference is negligible. — ryuu9187
– ryuu9187, Commented Oct 2, 2015 at 14:45
@fabian You're right - I was hasty (lazy) in choosing a dupe target. I find it difficult to believe that nobody has asked the questions "Is is better to use StringBuilder in a loop?" and "How do I use StringBuilder?" Google is plastered with well-written answers to both of these questions. — Rainbolt
– Rainbolt, Commented Oct 2, 2015 at 15:20

Pshemo · Accepted Answer · 2017-06-26 17:06:14Z

Code like

result = result + newPart;

is essentially (assuming result is String)

result = new StringBuilder(result).append(newPart).toString();

If you place it in loop, in each iteration you will be creating new StringBuilder object which will copy all characters from result String, then append newPart to it, and then create another String which will be stored in result variable.

This is very inefficient in case of long strings, because to add even small part we need to copy all characters from current result (twice, once when creating StringBuilder, and again when creating new result String).

To avoid this problem instead of string concatenation like

String result = "";
for (String newPart : otherStrings){
    result = result + newPart;
    result = result + ", ";
}

we should create one StringBuilder and append new parts to id. Advantage of StringBuilder class is that it holds characters in array of characters which is quite big, so it has free space for new ones. If that space at some point will be not enough StringBuilder will create new array with twice as big size and copy all characters there. So it limits amount of times we need to resize array and iterate over all characters.

So proper solution would look like:

StringBuilder sb = new StringBuilder();
for (String newPart : otherStrings){
    sb.append(newPart);
    sb.append(", ");
    //you could also chain these methods
    //sb.append(newPart).append(", ");
}
String result = sb.toString();

BTW StringBuilder allows manipulation on held characters. We can for instance remove some of its characters via StringBuilder.delete(int start, int end).

user3437460 · Accepted Answer · 2015-10-02 14:39:00Z

1

You can try something like this:

StringBuilder sb = new StringBuilder();
for(String[] sort : sorts){
    //Your other logic
    sb.append(sort[0]);
    sb.append(" ");
    sb.append(sort[1]);
    sb.append(",");
}

edited Oct 2, 2015 at 14:39

answered Oct 2, 2015 at 14:28

user3437460

17.5k18 gold badges67 silver badges110 bronze badges

Comments

NorthernSky · Accepted Answer · 2015-10-02 14:49:59Z

A straight conversion from String concatenation to using StringBuilder:

private String generateSuffix(List<String[]> searches, List<String[]> sorts) {
    StringBuilder ret = new StringBuilder();
    if (searches != null && !searches.isEmpty()) {
        ret.append("where ");
        for (String[] search : searches) {
            if (search.length < 3) {
                continue;
            }
            ret.append(search[0]).append(search[1]).append(search[2]).append(" and ");
        }
        ret.delete(ret.length() - 5, ret.length());
    }

    ret.append(" order by ");
    if (sorts != null && !sorts.isEmpty()) {
        for (String[] sort : sorts) {
            if (sort.length < 2) {
                continue;
            }
            ret.append(sort[0]).append(" ").append(sort[1]).append(",");
        }
    }
    return ret.toString();
}

Collectives™ on Stack Overflow

using stringbuilder with for loop

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related