17

In my project I am looping across a dataview result.

 string html =string.empty;
 DataView dV = data.DefaultView;
 for(int i=0;i< dV.Count;i++)
 {
     DataRowView rv = dV[i];
     html += rv.Row["X"].Tostring();
 }

Number of rows in dV will alway be 3 or 4.

Is it better to use the string concat += opearator or StringBuilder for this case and why?

Niraj Choubey
  • 3,942
  • 18
  • 58
  • 93
  • 2
    I think that there are many results about this on SO and on google. – Aristos Jun 21 '12 at 07:09
  • @Aristos why isn't it? In C# the operator `+` for `string`s are string concats – Alvin Wong Jun 21 '12 at 07:12
  • 1
    Either way don't think you would end up with significant performance edge, so just focus on readability. – V4Vendetta Jun 21 '12 at 07:12
  • @Aristos why joining two string into the new one is not concatination? please define concatenation then. – ie. Jun 21 '12 at 07:14
  • 1
    @Aristos is fast when you concat 1000 strings, but when only 3? – ie. Jun 21 '12 at 07:17
  • @Aristos you are very wrong, just write a sample and check the performance – ie. Jun 21 '12 at 07:19
  • @Aristos Hope now we are on the same page – V4Vendetta Jun 21 '12 at 07:20
  • @Aristos just run over this sample https://gist.github.com/2964397 – ie. Jun 21 '12 at 07:30
  • @ie. I do not have to run it, I understand right way the results. Here the string is converted to an optimized String.Concat(a,b,c) - is not the same with the += in a loop. Also there is an overhead to made the stringbuilder and the conversion to string. – Aristos Jun 21 '12 at 07:33
  • @Aristos that is what I was talking about, __always use the `StringBuilder`, it is faster in all cases__ is not the right answer – ie. Jun 21 '12 at 07:35

6 Answers6

41

I would use StringBuilder here, just because it describes what you're doing.

For a simple concatenation of 3 or 4 strings, it probably won't make any significant difference, and string concatenation may even be slightly faster - but if you're wrong and there are lots of rows, StringBuilder will start getting much more efficient, and it's always more descriptive of what you're doing.

Alternatively, use something like:

string html = string.Join("", dv.Cast<DataRowView>()
                                .Select(rv => rv.Row["X"]));

Note that you don't have any sort of separator between the strings at the moment. Are you sure that's what you want? (Also note that your code doesn't make a lot of sense at the moment - you're not using i in the loop. Why?)

I have an article about string concatenation which goes into more detail about why it's worth using StringBuilder and when.

EDIT: For those who doubt that string concatenation can be faster, here's a test - with deliberately "nasty" data, but just to prove it's possible:

using System;
using System.Diagnostics;
using System.Text;

class Test
{
    static readonly string[] Bits = { 
        "small string",
        "string which is a bit longer",
        "stirng which is longer again to force yet another copy with any luck"
    };

    static readonly int ExpectedLength = string.Join("", Bits).Length;

    static void Main()        
    {
        Time(StringBuilderTest);
        Time(ConcatenateTest);
    }

    static void Time(Action action)
    {
        GC.Collect();
        GC.WaitForPendingFinalizers();
        GC.Collect();
        // Make sure it's JITted
        action();
        Stopwatch sw = Stopwatch.StartNew();
        for (int i = 0; i < 10000000; i++)
        {
            action();
        }
        sw.Stop();
        Console.WriteLine("{0}: {1} millis", action.Method.Name,
                          (long) sw.Elapsed.TotalMilliseconds);
    }

    static void ConcatenateTest()
    {
        string x = "";
        foreach (string bit in Bits)
        {
            x += bit;
        }
        // Force a validation to prevent dodgy optimizations
        if (x.Length != ExpectedLength)
        {
            throw new Exception("Eek!");
        }
    }

    static void StringBuilderTest()
    {
        StringBuilder builder = new StringBuilder();
        foreach (string bit in Bits)
        {
            builder.Append(bit);
        }
        string x = builder.ToString();
        // Force a validation to prevent dodgy optimizations
        if (x.Length != ExpectedLength)
        {
            throw new Exception("Eek!");
        }
    }
}

Results on my machine (compiled with /o+ /debug-):

StringBuilderTest: 2245 millis
ConcatenateTest: 989 millis

I've run this several times, including reversing the order of the tests, and the results are consistent.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • @Aristos: Yes, that code will end up as `html = string.Concat(html, dV.Row["X"].ToString();` – Jon Skeet Jun 21 '12 at 07:15
  • 1
    @JonSkeet: the Microsoft documentation states that a String class is preferable for a concatenation operation if a fixed number of String objects are concatenated. – CloudyMarble Jun 21 '12 at 07:18
  • @O.D Can you give the reference of your statement? – Talha Jun 21 '12 at 07:30
  • 2
    @O.D: This isn't a fixed number though - it's in a loop. Basically it's worth using string concatenation if you can express everything in a single expression. That isn't the case here. – Jon Skeet Jun 21 '12 at 07:32
  • @Talha: the reference is in my answer – CloudyMarble Jun 21 '12 at 07:35
  • @Jon basically your right, but what about 10 or 20 concats in a foreach loop? is this s a fixed number? – CloudyMarble Jun 21 '12 at 07:39
  • 4
    @O.D: No - if it's in a loop, it's not fixed. In particular, it can't be represented in a single expression (and therefore a single call to Concat). – Jon Skeet Jun 21 '12 at 07:41
  • string concatenation may even be slightly faster???? how we can say that? any reference document plz – Talha Jun 21 '12 at 07:47
  • 2
    @JonSkeet: As someone who just bought your book yesterday i dont really dare argueing with you in C# issues, but take alook at this comparision: http://www.codeproject.com/Articles/14936/StringBuilder-vs-String-Fast-String-Operations-wit – CloudyMarble Jun 21 '12 at 07:48
  • @O.D: It depends on the sizes of the strings being concatenated and various other factors. But yes, it absolutely can work that way. – Jon Skeet Jun 21 '12 at 07:50
  • 1
    @Talha: String concatenation can be faster if the lengths of the strings involved mean that the `StringBuilder` has to copy the data on each concatenation anyway. There's the additional overhead of having a `StringBuilder` object to start with, and the call to `ToString` makes an additional copy. Note that the implementation of `StringBuilder` has changed significantly between .NET 3.5 and .NET 4, but I'd still expect it to *sometimes* be faster to use string concatenation for three or four strings. Not that I'm recommending it, you note. – Jon Skeet Jun 21 '12 at 07:52
  • @JonSkeet :) great. Now, its clear like crystal.... But how we determined the number of strings better for concatenation? – Talha Jun 21 '12 at 08:07
  • @JonSkeet: wait a minute, i thought you recommend the stringbuilder but the test you wrote proves that concatination is faster, am i write or did i missed anything? – CloudyMarble Jun 21 '12 at 08:24
  • 1
    @O.D: You missed the first few lines of my answer :) 1) The difference is unlikely to be significant in the grand scheme of things. 2) If the number of strings increases, `StringBuilder` *will* win. 3) The `StringBuilder` code makes it clearer what's going on. – Jon Skeet Jun 21 '12 at 08:52
  • 2
    @Talha: You don't. You recognize that in the cases where string concatenation in a loop is faster, the difference is almost always irrelevant - so you write the cleaner code which *also* scales up well. – Jon Skeet Jun 21 '12 at 08:53
  • hmm ok. anyway very nice and simple answers i must say. thank you jon :) – Talha Jun 21 '12 at 09:34
  • Jon Skeet's answers always teach me something. I like that. – rbedger Feb 17 '16 at 12:48
5

StringBuilder is recommended.. why dont you do an analysis for yourself and then decide what is the best for you..

var stopWatch=new StopWatch();
stopWatch.Start();
string html =string.empty;
        DataView dV = data.DefaultView;
        for(int i=0;i< dV.Count;i++)
        {
           html += dV.Row["X"].Tostring();
        } 
stopWatch.Stop();
Console.Write(stopWatch.EllapsedMilliseconds());

var stopWatch=new StopWatch();
stopWatch.Start();
string html =new StringBuilder();
        DataView dV = data.DefaultView;
        for(int i=0;i< dV.Count;i++)
        {
           html.Append(dV.Row["X"].ToString());
        } 
var finalHtml=html.ToString();
stopWatch.Stop();
Console.Write(stopWatch.EllapsedMilliseconds());
Baz1nga
  • 15,485
  • 3
  • 35
  • 61
  • 1
    I did .Both way took 1 miiliseconds . – Niraj Choubey Jun 21 '12 at 07:15
  • just what i wanted to suggest, and yep up to 5-10 loop times u wont see any dif, just try looping 1, 5, 10 15, ect and look at elapsedTicks – bresleveloper Jun 21 '12 at 07:16
  • bresleveloper - I know StringBuilder is efficient for string concatenation if there are large or random number of string to concat. But i want to know if we have 3 or 4 strings to concat then in this case which is better. – Niraj Choubey Jun 21 '12 at 07:22
  • 3
    @NirajChoubey: Do you have any evidence that the *performance* of this code is relevant? It's almost always better to go for readability - and `StringBuilder` expresses your intentions much more clearly here. – Jon Skeet Jun 21 '12 at 07:49
2

From the Documentation:

The String class is preferable for a concatenation operation if a fixed number of String objects are concatenated. In that case, the individual concatenation operations might even be combined into a single operation by the compiler.

A StringBuilder object is preferable for a concatenation operation if an arbitrary number of strings are concatenated; for example, if a loop concatenates a random number of strings of user input.

So in your case i would say the String is better.

EDIT:

This is a no end disscussion, anyway i would recommend you to check how many opaeration do you have in average and test the performance for each one of them to compare results.

Check this nice link regarding this issue including some performance test code.

CloudyMarble
  • 36,908
  • 70
  • 97
  • 130
  • 1
    I think that by `...fixed number of String objects are concatenated...` was meant something like `"abc" + "def" + "ghi"` and not `for` loop. – Betlista Jun 21 '12 at 07:27
  • What if i write a for loop to concat 3 strings? would it make any difference? no so what about 10 strings? or 20? the loop is not the issue, see my edit, i think the OP should know in average how many operations are involved and test the performance of each option on this amount of concats – CloudyMarble Jun 21 '12 at 07:42
  • Probably you have no idea how compiler works. If the compiler see `"abc" + "def" + "ghi"` in code, it's possible to combine this into single operation as it is written in your post, but for typical loop compiler do NOT know number of cycles and there will be operation for each cycle in loop. That's also the reason why [loop unwinding](http://en.wikipedia.org/wiki/Loop_unwinding) is performance optimization. Simply `if a fixed number of String objects are concatenated` condition doesn't hold, because compiler do not know that there are always 3 or 4 items even if you know that... – Betlista Jun 21 '12 at 09:02
  • But in a loop from 0 to 99 we do know how many operations are involved, and 100 is a fixed number isnt it? i think the condition does hold in this case, it depends much more on how long each string is and not on if the number of loops is known or not, stringbuilder may spare memnroy but its not allways a faster option, c the performance test example. – CloudyMarble Jun 21 '12 at 09:49
1

StringBuilder for sure. String are immutable remember !

EDIT: For 3-4 rows, concatenation will be a preferred choice as Jon Skeet has said in his answer

Habib
  • 219,104
  • 29
  • 407
  • 436
  • For a small number of strings, it could actually be slower than string concatenation. In particular, if the `StringBuilder` needs to expand on each `Append`, you'll still have all the copying overhead, but you *also* have the final copying operation and the `StringBuilder` itself. – Jon Skeet Jun 21 '12 at 07:14
  • @JonSkeet, Yes Jon, I just saw the "3-4 rows" part in the question. Before the edit it was part of the code and I missed it – Habib Jun 21 '12 at 07:17
1

StringBuilder is recommended. It is mutable. It should place much less stress on the memory allocator :-)

A string instance is immutable. You cannot change it after it was created. Any operation that appears to change the string instead returns a new instance.

Talha
  • 18,898
  • 8
  • 49
  • 66
0

stringbuilder is what you are looking for. In general, if there is a function for some job try to utilize it instead of writing some procedure which does pretty much the same job.

Ozgur Dogus
  • 911
  • 3
  • 14
  • 38