views:

129

answers:

2

It's common knowledge that you shouldn't use a StringBuilder in place of a small number of concatenations:

string s = "Hello";
if (greetingWorld)
{
    s += " World";
}

s += "!";

However, in loops of a significant size, StringBuilder is the obvious choice:

string s = "";
foreach (var i in Enumerable.Range(1,5000))
{
    s += i.ToString(); // <- bad idea!
}

Console.WriteLine(s);

Is there a tool that I can run on either raw C# source or a compiled assembly to identify where in the source code that String.Concat is being called? (If you're not familiar, s += "foo" is mapped to String.Concat in the IL output.) Obviously, I can't realistically search through an entire project and evaluate every += to identify whether the lvalue is a string.

Ideally, it would only point out calls inside a for/foreach loop, but I would even put up with all the false positives of noting every String.Concat. Also, I'm aware that there are some refactoring tools that will automatically refactor my code to use StringBuilder, but I am only interested in identifying the Concat usage at this point.

I routinely run Gendarme and FxCop on my code, and neither of those tools identify what I've described. However, as @Cristian pointed out, older versions of FxCop used to check for this. Maybe there's a way to extract just that rule from an old version of FxCop and tell the newer version (1.36) to use it?

+3  A: 

Perhaps NDepend CQL (Code Query Language) is expressive enough for this. Not sure if it is though.

Oded
A: 

FxCop had some advices for that. Check this article

For instance according to the article in this code:

static string BadConcatenate(string[] items)
{
    string strRet = string.Empty;

    foreach(string item in items)
    {
        strRet += item;
    }

    return strRet;
}

FxCop reports

"Change StringCompareTest.BadConcatenate(String[]):String to use StringBuilder 
  instead of String.Concat or +

Edit

It looks like the rule CA1807 has been removed either because of high noise or no longer applicable analysis. And it looks like the compiler is not automatically replacing it, in the same link they elaborate more on the performance of both methods.

Cristian
Apparently the reason I never saw this was that [the particular rule was removed in 2007 or earlier](http://blogs.msdn.com/codeanalysis/archive/2007/08/09/what-rules-do-microsoft-have-turned-on-internally.aspx#5390704). *"Unfortunately, these rules were based on our old data flow engine and were removed when it was removed from the product. We will be recovering them in a future version of Visual Studio."*
Mark Rushakoff