HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

StringBuilder vs string.concat

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
stringbuilderstringconcat

Problem

I was reading this article about StringBuilder vs string.concat and couldn't figure out which one would be better in the following situation:

StringBuilder address = new StringBuilder();

address.Append(retailer.Street.Replace(",", "").Replace("\n", ""));
address.Append("");

if (!string.IsNullOrWhiteSpace(retailer.City)) 
{
  address.Append(retailer.City);
  address.Append("");
}

if (!string.IsNullOrWhiteSpace(retailer.County)) 
{
  address.Append(retailer.County);
  address.Append("");
}

address.Append(retailer.Postcode);

return MvcHtmlString.Create(address.ToString());


According to the article, as it is not in a loop, I should be using string.Concat. But as there are if statements while building the string, I was wondering if that still applied.

Or would I be better off doing the following:

string address = string.Concat(retailer.Street.Replace(",", "").Replace("\n", ""), "");

if (!string.IsNullOrWhiteSpace(retailer.City))
{
    address += string.Concat(retailer.City, "");
}

if (!string.IsNullOrWhiteSpace(retailer.County))
{
    address += string.Concat(retailer.County, "");
}

address += retailer.Postcode;

return MvcHtmlString.Create(address);

Solution

DISCLAIMER: You haven't properly defined "better" so I'll assume you're talking about performance

You're fixing the wrong problem.

Under the assumption that your retailer is an instance of some Retailer POCO, you should properly normalize your data and extract the Address data into a separate Table / POCO.

Then you can stop generating HTML in some Controller code and instead do something like:

retailer.Address.ToHTMLString();


This is significantly cleaner and allows central modification of "ToHTML" rules.

In addition to that I don't think you actually have to care about the performance difference between these two. Have you profiled your code and proved that this is the bottleneck? If it isn't.... It's good enough I'd say.

You seem to be falling prey to the problems described in Eric Lippert's Performance Rant

Code Snippets

retailer.Address.ToHTMLString();

Context

StackExchange Code Review Q#120856, answer score: 8

Revisions (0)

No revisions yet.