8

I'm writing a camera control program which uses many methods for each of the different commands.

One example is:

public void CameraPan(int Id, string Direction, int Speed)
{
    StringBuilder sb = new StringBuilder();
    sb.Append(_cameraRamp); //command to camera - start moving
    sb.Append(_camID);
    sb.Append(Convert.ToString(Id));
    sb.Append(_pan); //command to camera - this is a pan
    sb.Append(Direction);
    sb.Append(_panSpeed); //command to camera - speed to move at
    sb.Append(Convert.ToString(Speed));
    _sendCommand = sb.ToString();
    SendToCamera(_sendCommand);
}

My question - is it best practice to instantiate a new StringBuilder for each method or construct one StringBuilder for the class and use it throughout?

It seems to me that if I use one throughout there would be fewer resources committed. On the other hand, wouldn't the GC just dispose of each instance when the method is no longer being used?

1
  • 4
    As a side note, you can chain the Append() calls to condense the code a little; it's a fluent interface. See this question for examples.
    – DylanSp
    Mar 16 '16 at 20:07
18

Instantiate a StringBuilder for each method.

Here's why:

  1. Encapsulation. Because each method has its own StringBuilder object that it controls, it does not have to worry about anything else monkeying with its StringBuilder state.

  2. StringBuilders are cheap objects to instantiate, much cheaper than concatenating strings.

Sharing a global StringBuilder object will create a fertile breeding ground for all sorts of subtle, insidious, difficult to troubleshoot bugs, especially if you're trying to stay thread-safe.

3
  • 3
    I'm not familiar with the GC model of the CLR but if it's generational, another reason is that sort lived objects are really inexpensive to allocate and collect. Something that lives for a while will end up in a mark-sweep-compact generation and actually be more costly from a resource perspective.
    – JimmyJames
    Mar 16 '16 at 20:03
  • 4
    Moreover on #1, this it a multi-threading no-no.
    – JimmyJames
    Mar 16 '16 at 20:06
  • 1
    As a matter of fact, there is a StringBuilderCache that is used by string.format. The cache makes use of the ThreadStatic attribute that ties a static to a Thread, and not to an AppDomain. The cache also reuses the largest instance that was previously neccesary, avoiding growth issues inside the builder. Mar 23 '16 at 8:07
6

Use a new StringBuilder whenever you need a new StringBuilder.

Anything else is probably breaking encapsulation and leaves you open to the undesired side-effects of sharing an object.

Also, I'd suggest using AppendFormat() instead of plain Append().
You're constructing a fairly complex (and, I assume, format-sensitive) string here, so locking down the formatting explicitly is going to save you headaches later on.

sb.AppendFormat("{0}{1}{2:0}{3}{4}{5:0}" 
   , _cameraRamp  /* start moving */ 
   , _camID
   , Id 
   , _pan         /* this is a pan */ 
   , Direction  
   , _panSpeed    /* speed to move at */ 
   , Speed 
   ); 

_sendCommand = sb.ToString();

SendToCamera(_sendCommand);
3
  • That's a great tip, Phill. Since I've just starting writing this class, I'll incorporate the idea. Mar 18 '16 at 15:11
  • This places emphasis on the whole string; however be sure that the parameter list matches the arguments - the last parameter is superfluous. I don't know whether someone wrote a diagnostic for that; I'm sure R# would warn about it. Mar 23 '16 at 8:02
  • 1
    In which case you are better off using string.Format directly. No need to create a StringBuilder yourself.
    – Kryptos
    Jan 23 '19 at 0:29
0

In this specific scenario I don't think you will gain much using a StringBuilder. You would probably get the same performance with string interpolation as follows:

SendToCamera($"{_cameraRamp}{_camID}{Id}{_pan}{Direction}{_panSpeed}{Speed}");

As you are concatenating very few strings, and not in a loop.

From the documentation:

Although the StringBuilder class generally offers better performance than the String class, you should not automatically replace String with StringBuilder whenever you want to manipulate strings. Performance depends on the size of the string, the amount of memory to be allocated for the new string, the system on which your code is executing, and the type of operation. You should be prepared to test your code to determine whether StringBuilder actually offers a significant performance improvement.

You can take a look at https://docs.microsoft.com/en-us/dotnet/api/system.text.stringbuilder#the-string-and-stringbuilder-types

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.