CODEWITHSUNDEEP
c#.netstringbuilder
dlras2
dlras2

Reputation: 8486

Am I undermining the efficiency of StringBuilder?

I've started using StringBuilder in preference to straight concatenation, but it seems like it's missing a crucial method. So, I implemented it myself, as an extension:

public void Append(this StringBuilder stringBuilder, params string[] args)
{
    foreach (string arg in args)
        stringBuilder.Append(arg);
}

This turns the following mess:

StringBuilder sb = new StringBuilder();
...
sb.Append(SettingNode);
sb.Append(KeyAttribute);
sb.Append(setting.Name);

Into this:

sb.Append(SettingNode, KeyAttribute, setting.Name);

I could use sb.AppendFormat("{0}{1}{2}",..., but this seems even less preferred, and still harder to read. Is my extension a good method, or does it somehow undermine the benefits of StringBuilder? I'm not trying to prematurely optimize anything, as my method is more about readability than speed, but I'd also like to know I'm not shooting myself in the foot.

Upvotes: 52

Views: 2857

Answers (10)

Ben Voigt
Ben Voigt

Reputation: 283624

Potentially even faster, because it performs at most one reallocation/copy step, for many appends.

public void Append(this StringBuilder stringBuilder, params string[] args)
{
    int required = stringBuilder.Length;
    foreach (string arg in args)
        required += arg.Length;
    if (stringBuilder.Capacity < required)
        stringBuilder.Capacity = required;
    foreach (string arg in args)
        stringBuilder.Append(arg);
}

Upvotes: 1

Matthew Sposato
Matthew Sposato

Reputation: 1635

Chris,

Inspired by this Jon Skeet response (second answer), I slightly rewrote your code. Basically, I added the TestRunner method which runs the passed-in function and reports the elapsed time, eliminating a little redundant code. Not to be smug, but rather as a programming exercise for myself. I hope it's helpful.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Diagnostics;

namespace SBTest
{
  class Program
  {
    private static void Main(string[] args)
    {
      // JIT everything
      AppendTest(1);
      AppendFormatTest(1);

      int iterations = 1000000;

      // Run Tests
      TestRunner(AppendTest, iterations);
      TestRunner(AppendFormatTest, iterations);

      Console.ReadLine();
    }

    private static void TestRunner(Func<int, long> action, int iterations)
    {
      GC.Collect();

      var sw = Stopwatch.StartNew();
      long length = action(iterations);
      sw.Stop();

      Console.WriteLine("--------------------- {0} -----------------------", action.Method.Name);
      Console.WriteLine("iterations: {0:n0}", iterations);
      Console.WriteLine("milliseconds: {0:n0}", sw.ElapsedMilliseconds);
      Console.WriteLine("output length: {0:n0}", length);
      Console.WriteLine("");
    }

    private static long AppendTest(int iterations)
    {
      var sb = new StringBuilder();

      for (var i = 0; i < iterations; i++)
      {
        sb.Append("TEST" + i.ToString("00000"),
                  "TEST" + (i + 1).ToString("00000"),
                  "TEST" + (i + 2).ToString("00000"));
      }

      return sb.Length;
    }

    private static long AppendFormatTest(int iterations)
    {
      var sb = new StringBuilder();

      for (var i = 0; i < iterations; i++)
      {
        sb.AppendFormat("{0}{1}{2}",
            "TEST" + i.ToString("00000"),
            "TEST" + (i + 1).ToString("00000"),
            "TEST" + (i + 2).ToString("00000"));
      }

      return sb.Length;
    }
  }

  public static class SBExtentions
  {
    public static void Append(this StringBuilder sb, params string[] args)
    {
      foreach (var arg in args)
        sb.Append(arg);
    }
  }
}

Here's the output:

--------------------- AppendTest -----------------------
iterations: 1,000,000
milliseconds: 1,274
output length: 29,700,006

--------------------- AppendFormatTest -----------------------
iterations: 1,000,000
milliseconds: 1,381
output length: 29,700,006

Upvotes: 0

Jason
Jason

Reputation: 951

I haven't tested recently, but in the past, StringBuilder was actually slower than plain-vanilla string concatenation ("this " + "that") until you get to about 7 concatenations.

If this is string concatenation that is not happening in a loop, you may want to consider if you should be using the StringBuilder at all. (In a loop, I start to worry about allocations with plain-vanilla string concatenation, since strings are immutable.)

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1499790

It's a little bit of overhead creating the extra array, but I doubt that it's a lot. You should measure

If it turns out that the overhead of creating string arrays is significant, you can mitigate it by having several overloads - one for two parameters, one for three, one for four etc... so that only when you get to a higher number of parameters (e.g. six or seven) will it need to create the array. The overloads would be like this:

public void Append(this builder, string item1, string item2)
{
    builder.Append(item1);
    builder.Append(item2);
}

public void Append(this builder, string item1, string item2, string item3)
{
    builder.Append(item1);
    builder.Append(item2);
    builder.Append(item3);
}

public void Append(this builder, string item1, string item2,
                   string item3, string item4)
{
    builder.Append(item1);
    builder.Append(item2);
    builder.Append(item3);
    builder.Append(item4);
}

// etc

And then one final overload using params, e.g.

public void Append(this builder, string item1, string item2,
                   string item3, string item4, params string[] otherItems)
{
    builder.Append(item1);
    builder.Append(item2);
    builder.Append(item3);
    builder.Append(item4);
    foreach (string item in otherItems)
    {
        builder.Append(item);
    }
}

I'd certainly expect these (or just your original extension method) to be faster than using AppendFormat - which needs to parse the format string, after all.

Note that I didn't make these overloads call each other pseudo-recursively - I suspect they'd be inlined, but if they weren't the overhead of setting up a new stack frame etc could end up being significant. (We're assuming the overhead of the array is significant, if we've got this far.)

Upvotes: 9

user98071
user98071

Reputation: 21

From a clarity perspective, your extension is ok.

It would probably be best to simply use the .append(x).append(y).append(z) format if you never have more than about 5 or 6 items.

StringBuilder itself would only net you a performance gain if you were processing many thousands of items. In addition you'll be creating the array every time you call the method.

So if you're doing it for clarity, that's ok. If you're doing it for efficiency, then you're probably on the wrong track.

Upvotes: 2

Chris
Chris

Reputation: 28064

Questions like this can always be answered with a simple test case.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Diagnostics;

namespace SBTest
{
    class Program
    {
        private const int ITERATIONS = 1000000;

        private static void Main(string[] args)
        {
            Test1();
            Test2();
            Test3();
        }

        private static void Test1()
        {
            var sw = Stopwatch.StartNew();
            var sb = new StringBuilder();

            for (var i = 0; i < ITERATIONS; i++)
            {
                sb.Append("TEST" + i.ToString("00000"),
                          "TEST" + (i + 1).ToString("00000"),
                          "TEST" + (i + 2).ToString("00000"));
            }

            sw.Stop();
            Console.WriteLine("Testing Append() extension method...");
            Console.WriteLine("--------------------------------------------");
            Console.WriteLine("Test 1 iterations: {0:n0}", ITERATIONS);
            Console.WriteLine("Test 1 milliseconds: {0:n0}", sw.ElapsedMilliseconds);
            Console.WriteLine("Test 1 output length: {0:n0}", sb.Length);
            Console.WriteLine("");
        }

        private static void Test2()
        {
            var sw = Stopwatch.StartNew();
            var sb = new StringBuilder();

            for (var i = 0; i < ITERATIONS; i++)
            {
                sb.Append("TEST" + i.ToString("00000"));
                sb.Append("TEST" + (i+1).ToString("00000"));
                sb.Append("TEST" + (i+2).ToString("00000"));
            }

            sw.Stop();    
            Console.WriteLine("Testing multiple calls to Append() built-in method...");
            Console.WriteLine("--------------------------------------------");
            Console.WriteLine("Test 2 iterations: {0:n0}", ITERATIONS);
            Console.WriteLine("Test 2 milliseconds: {0:n0}", sw.ElapsedMilliseconds);
            Console.WriteLine("Test 2 output length: {0:n0}", sb.Length);
            Console.WriteLine("");
        }

        private static void Test3()
        {
            var sw = Stopwatch.StartNew();
            var sb = new StringBuilder();

            for (var i = 0; i < ITERATIONS; i++)
            {
                sb.AppendFormat("{0}{1}{2}",
                    "TEST" + i.ToString("00000"),
                    "TEST" + (i + 1).ToString("00000"),
                    "TEST" + (i + 2).ToString("00000"));
            }

            sw.Stop();
            Console.WriteLine("Testing AppendFormat() built-in method...");
            Console.WriteLine("--------------------------------------------");            
            Console.WriteLine("Test 3 iterations: {0:n0}", ITERATIONS);
            Console.WriteLine("Test 3 milliseconds: {0:n0}", sw.ElapsedMilliseconds);
            Console.WriteLine("Test 3 output length: {0:n0}", sb.Length);
            Console.WriteLine("");
        }
    }

    public static class SBExtentions
    {
        public static void Append(this StringBuilder sb, params string[] args)
        {
            foreach (var arg in args)
                sb.Append(arg);
        }
    }
}

On my PC, the output is:

Testing Append() extension method...
--------------------------------------------
Test 1 iterations: 1,000,000
Test 1 milliseconds: 1,080
Test 1 output length: 29,700,006

Testing multiple calls to Append() built-in method...
--------------------------------------------
Test 2 iterations: 1,000,000
Test 2 milliseconds: 1,001
Test 2 output length: 29,700,006

Testing AppendFormat() built-in method...
--------------------------------------------
Test 3 iterations: 1,000,000
Test 3 milliseconds: 1,124
Test 3 output length: 29,700,006

So your extension method is only slightly slower than the Append() method and is slightly faster than the AppendFormat() method, but in all 3 cases, the difference is entirely too trivial to worry about. Thus, if your extension method enhances the readability of your code, use it!

Upvotes: 32

Jimmy Hoffa
Jimmy Hoffa

Reputation: 5967

I wouldn't say you're undermining it's efficiency, but you may be doing something inefficient when a more efficient method is available. AppendFormat is what I think you want here. If the {0}{1}{2} string being used constantly is too ugly, I tend to put my format strings in consts above, so the look would be more or less the same as your extension.

sb.AppendFormat(SETTING_FORMAT, var1, var2, var3);

Upvotes: 1

Benjamin Anderson
Benjamin Anderson

Reputation: 1384

Ultimately it comes down to which one results in less string creation. I have a feeling that the extension will result in a higher string count that using the string format. But the performance probably won't be that different.

Upvotes: 0

Jesper Palm
Jesper Palm

Reputation: 7238

I see no problem with your extension. If it works for you it's all good.

I myself prefere:

sb.Append(SettingNode)
  .Append(KeyAttribute)
  .Append(setting.Name);

Upvotes: 69

KP.
KP.

Reputation: 13730

Other than a bit of overhead, I don't personally see any issues with it. Definitely more readable. As long as you're passing a reasonable number of params in I don't see the problem.

Upvotes: 3

Related Questions