Multiple StringBuilders inside StringBuilder. Does it worth?

Question

I receive a list of models. The number of models could be large. This models has a bunch of properties and any of them could be null potentially.

I need to build a string for every model based of it's properties. If property == null then I add some static part to the result string like "property1 is null".

If else property != null then I add something like this "property1 == 'valueOfThePropertyHere'".

The result string should look something like this: prop1 == 'value1' and prop2 is null and prop3 == 'value3' and prop4 == 'value4' and prop5 is null and ..... propN == 'valueN'

And I generate such string for every model from the list.

Obviously I do this in for loop and I use StringBuilder for this. The thing is that in append method of StringBuilder I check every field of the model for null using ternary operator and based on this I add the result of this check to the result string. But if a property is not null then I need to add some static part + value of the field itself + some more static stuff. And that means I need to add one more StringBuilder for every property I have. Or I can use '+' which will be transformed into StringBuilder anyway and as far as I know it's a bad practise to use '+' inside StringBuilder (but I have to use it anyway).

Example:

List<Model> models = repository.getModels();

for (Model m: models) {
    StringBuilder stringBuilder = new StringBuilder();

    stringBuilder
    .append(m.getField1() == null ? "field1  is null" : "field1 == '" + new StringBuiler().append(m.getField1()).append("'").append(" and ").toString()))
    .append(m.getField2() == null ? "field2  is null" : "field2 == '" + new StringBuiler().append(m.getField2()).append("'").append(" and ").toString()))
    ...............
    .append(m.getFieldN() == null ? "fieldN  is null" : "fieldN == '" + new StringBuiler().append(m.getFieldN()).append("'").append(" and ").toString()));

    System.out.println(stringBuilder.toString());
    }

In my opinion from the performance perspective it doesn't look so well because for every model from a list of models I create another bunch of StringBuilder objects in heap just to get the result string.

Am I missing something? Are there better ways to do so from the performance perspective? Or it's okay because I don't see other options for now.

https://stackoverflow.com/questions/513600/should-i-use-javas-string-format-if-performance-is-important has some information regarding the available options and their speed. — kendavidson, Nov 08 '19 at 15:00
*"In my opinion from the performance perspective it doesn't look so well"* I disagree because the performance perspective here is so incredibly microscopic that you'll have trouble even measuring any differences. Let the compiler + JVM + hotspot worry about micro optimizations, you just write good, clean code. — Gimby, Nov 08 '19 at 15:01
Creating a new StringBuilder just to immediately append 3 things and call toString() is defeating the purpose of the StringBuilder — RobOhRob, Nov 08 '19 at 15:03
Just use `if/else`, and append directly to the main `StringBuilder`. — Andy Turner, Nov 08 '19 at 15:03
How many models might you receive? Generally programmers optimize prematurely, and should code for clarity. — AndyMan, Nov 08 '19 at 15:08
method getting the `StringBuilder` and `if` inside to avoid another concatenation, I would say... (short methods are very easy for JIT to inline) - despite in newer Java versions tests are required: string concatenation is not done with `StringBuilder`anymore — user85421, Nov 08 '19 at 15:16
BTW `new StringBuilder().append(a).append(b)...toString()` is what compiler did for `a+b+...` (but later is easier to read) — user85421, Nov 08 '19 at 15:18
@Coffeemanz Just a 1000? Go for simple and clear. If you're worried, add a log statement with the time it took. — AndyMan, Nov 08 '19 at 15:32

Andy Turner · Accepted Answer · 2019-11-08T15:25:52.123

4

Go for simple.

Instead of

stringBuilder
.append(m.getField1() == null ? "field1  is null" : "field1 == '" + new StringBuiler().append(m.getField1()).append("'").append(" and ").toString()))

use:

if (m.getField1() == null) {
  stringBuilder.append("field1  is null");
} else {
  stringBuilder.append("field1 == '").append(m.getField1()).append("'").append(" and ");
}

Aside from the distinct oddness of using a StringBuilder inside a StringBuilder.append call (and why not just use + anyway...), it's really hard to parse where the : is in the conditional expression. Breaking it into lines is much easier.

If you find yourself having to repeat this code pattern a lot, define a method:

void append(StringBuilder stringBuilder, String name, Object value) {
  stringBuilder.append(name);
  if (value == null) {
    stringBuilder.append(" is null");
  } else {
    stringBuilder.append(" == '").append(value).append("'").append(" and ");
  }
}

and then invoke like:

append(stringBuilder, "field1", m.getField1());
append(stringBuilder, "field2", m.getField2());
append(stringBuilder, "field3", m.getField3());

edited Nov 08 '19 at 15:25

answered Nov 08 '19 at 15:05

Andy Turner

137,514
11
162
243

Mind the missing "and" in the == null case. – Fildor Nov 08 '19 at 15:08
@AndyMan seems reasonable. Wanted to make it compact (if it's possible to say so in this case) as much as it's possible. – Coffemanz Nov 08 '19 at 15:15
@Coffemanz 'compact 'and 'performance perspective' are very different... – user85421 Nov 08 '19 at 15:21
If you want compact, then just drop the second StringBuilder and instead use strings like " == " + value + "' and ". Java uses StringBuilder internally so they are equivalent. – AndyMan Nov 08 '19 at 15:37

score 0 · Answer 2 · answered Nov 08 '19 at 15:15

What a mess! Just because you can chain invocations, doesn't mean you should:

List<Model> models = repository.getModels();

    for (Model m: models) {
        StringBuilder stringBuilder = new StringBuilder();

        String field = m.getField1();
        if(field==null) {
            stringBuilder.append("field1 is null");
        } else {
            stringBuilder.append("field1 == ").append(m.getField1()).append("'");
        }

        if(stringBuilder.length()>0) {
            stringBuilder.append(" and ");
        }

        field = m.getField2();
        if(field==null) {
            stringBuilder.append("field2 is null");
        } else {
            stringBuilder.append("field2 == ").append(m.getField1()).append("'");
        }

        if(stringBuilder.length()>0) {
            stringBuilder.append(" and ");
        }
        ...

        System.out.println(stringBuilder.toString());
    }

To avoid all this potential repetition (depending on number of fields):

void appendField(StringBuilder stringBuilder, String fieldName, String value) {
    if(stringBuilder.length()>0) {
        stringBuilder.append(" and ");
    }
    stringBuilder.append(fieldName);
    if(value==null) {
        stringBuilder.append(" is null");
    } else {
        stringBuilder.append(" == '").append(value).append("'");
    }
}
String toString(Model m) {
    StringBuilder stringBuilder = new StringBuilder();

    appendField(stringBuilder, "field1", m.getField1());
    appendField(stringBuilder, "field2", m.getField2());
    ...
    appendField(stringBuilder, "fieldN", m.getFieldN());

    return stringBuilder.toString();
}

List<Model> models = repository.getModels();

for (Model m: models) {
    System.out.println(toString(m));
}

Multiple StringBuilders inside StringBuilder. Does it worth?

2 Answers2