CODEWITHSUNDEEP
c#.netstringbuilder
apocalypse
apocalypse

Reputation: 5884

StringBuilder, append string if conditions are met

var sb = new StringBuilder ();

if (condition1) sb.Append ("one");
if (condition2) sb.Append ("two");
if (condition3) sb.Append ("three");
if (condition4) sb.Append ("four");
if (condition5) sb.Append ("five");

return sb.ToString ();

Any idea how to improve it? How to write less code, giving same result?

Upvotes: 10

Views: 15510

Answers (4)

Ömer
Ömer

Reputation: 1666

Another approach is to use string interpolation like documented here https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/tokens/interpolated. So, the example can be modified like this. I think it is easier to read.

var s = $"{(condition1 ? "one": null)}{(condition2 ? "two": null)}{(condition3 ? "three": null)}{(condition4 ? "four": null)}{(condition5 ? "five": null)}";

Edit: You can make the code easier to read by making it verbatim string literal.

var s = $@"{
    (condition1 ? "one": null)
}{
    (condition2 ? "two": null)
}{
    (condition3 ? "three": null)
}{
    (condition4 ? "four": null)
}{
    (condition5 ? "five": null)}";

Upvotes: 2

Uladz
Uladz

Reputation: 1968

I prefer approach with simple DSLs definition, when it makes code simpler or more readable. Also "pipeline-style" expressions are awesome. In your case it can be written like this:

var str =
    new StringBuilder()
        .AppendIf(condition1, "one")
        .AppendIf(condition2, "two")
        .AppendIf(condition3, "forty two")
        .ToString();

With an extension method.

public static class StringBuilderExtensions
{
    public static StringBuilder AppendIf(
        this StringBuilder @this,
        bool condition,
        string str)
    {
        if (@this == null)
        {
            throw new ArgumentNullException("this");
        }

        if (condition)
        {
            @this.Append(str);
        }

        return @this;
    }
}

Approach is suitable here, if conditions are repeated. For example arg1 != null, arg2 != null, then AppendIfNotNull can be used.

Otherwise, think twice, because it looks quite similar to initial implementation, requires additional code, can be slower because of additional null checks and method invocations, and also you should create an AppendIf overload for every Append one.

Upvotes: 8

Jodrell
Jodrell

Reputation: 35716

You could do something like,

var conditions = new[]
    {
        Tuple.Create(condition1, "one"),
        Tuple.Create(condition2, "two"),
        Tuple.Create(condition3, "three"),
        Tuple.Create(condition4, "four"),
        Tuple.Create(condition5, "five"),
    }

return string.Concat(conditions.Where(t => t.Item1).Select(t => t.Item2));

Is that better? No.

Upvotes: 3

Patrick Hofman
Patrick Hofman

Reputation: 156948

This code is fine. Keep it like this.

  • Every attempt to use extension methods or others will only make this code less understandable and maintainable;
  • There is no repeating code;
  • As long as the conditions don't influence another, there is no way of shortening the ifs.

If you do want another option:

string s = 
   (condition1 ? "one" : null) + 
   (condition2 ? "two" : null) + 
   (condition3 ? "three" : null) + 
   (condition4 ? "four" : null) + 
   (condition5 ? "five" : null)
   ;

But let's be honest, does this make it better? No.

Upvotes: 16

Related Questions