tags:

views:

150

answers:

4

I have a list of items that I wish to display with a separator between them in C#. Using a normal iterator I would end up with an extra separator at the beginning or the end:

string[] sa = {"one", "two", "three", "four"};
string ns = "";
foreach(string s in sa)
{
    ns += s + " * ";
}
// ns has a trailing *:
// one * two * three * four *

Now I can solve this using a for loop like so:

ns = "";
for(int i=0; i<sa.Length; i++)
{
    ns += sa[i];
    if(i != sa.Length-1)
     ns += " * ";
}
// this works:
// one * two * three * four

Although the second solution works it doesn't look very elegant. Is there a better way to do this?

+13  A: 

You need the built-in String.Join method:

string ns = string.Join(" * ", sa);

If you want to do the same with other collection types, then you can still use String.Join if you create an array first using LINQ's ToArray method:

string ns = string.Join(" * ", test.ToArray());
LukeH
Winston Smith
+1  A: 

The advantage of this approach is you can use it on any type of sequence, and not just a string array.

var ns = sa.Aggregate( (c, s) => c + " * " + s);
Winston Smith
Joe, Since you're using LINQ anyway, you'll get better performance and more readable code if you create an array from your sequence and then use String.Join. For example: string.Join(" * ", test.ToArray());
LukeH
Yes, one might project from a sequence of objects a certain field into a sequence of strings and use your method. I wasn't aware of string.Join before - a perfect example of why LINQ isn't a panacea :)
Winston Smith
A: 

I much prefer Luke's solution.

string ns = string.Join(" * ", sa);

Alternatively you could do this if your collection is not indexable but just enumerable:

string ns = "";
foreach(string s in sa)
{
    if (ns.Length != 0)
    {
        ns += " * ";
    }

    ns += s;
}

It's like your second example but it puts the test at the start of the loop and less likely to hit one-off errors like would be possible in your second example. Arrays are obviously indexable but in some cases you get containers (Namely System.Collections.Generic.Dictionary<T,K>.Values) that are not indexable and you'll want something like this instead.

Colin Burnett
+2  A: 

In addition to elegance you might want to consider speed and reusability across types other than String. For elegance I would suggest using an extension method to abstract away the details so that the common usage would look something like:

ns = sa.Join(" * ");

For speed consider the following variant tests including some of the solutions proposed by other people who have answered the question:

public void Test_variants()
{
    const string item = "a";
    const int numberOfTimes = 100000;
    const string delimiter = ", ";
    string[] items = new List<string>(Enumerable.Repeat(item, numberOfTimes)).ToArray();
    string expected = String.Join(delimiter, items);

    Time(StringJoin, items, delimiter, expected);
    Time(Aggregate, items, delimiter, expected);
    Time(CheckForEndInsideLoop_String, items, delimiter, expected);
    Time(CheckForBeginningInsideLoop_String, items, delimiter, expected);
    Time(RemoveFinalDelimiter_String, items, delimiter, expected);
    Time(CheckForEndInsideLoop_StringBuilder, items, delimiter, expected);
    Time(RemoveFinalDelimiter_StringBuilder, items, delimiter, expected);
}

private static void Time(Func<string[], string, string> func, string[] items, string delimiter, string expected)
{
    Stopwatch stopwatch = new Stopwatch();
    stopwatch.Start();
    string result = func(items, delimiter);
    stopwatch.Stop();
    bool isValid = result == expected;
    Console.WriteLine("{0}\t{1}\t{2}", stopwatch.Elapsed, isValid, func.Method.Name);
}

private static string CheckForEndInsideLoop_String(string[] items, string delimiter)
{
    string result = "";
    for (int i = 0; i < items.Length; i++)
    {
     result += items[i];
     if (i != items.Length - 1)
     {
      result += delimiter;
     }
    }
    return result;
}

private static string RemoveFinalDelimiter_String(string[] items, string delimiter)
{
    string result = "";
    for (int i = 0; i < items.Length; i++)
    {
     result += items[i] + delimiter;
    }
    return result.Substring(0, result.Length - delimiter.Length);
}

private static string CheckForBeginningInsideLoop_String(string[] items, string delimiter)
{
    string result = "";
    foreach (string s in items)
    {
     if (result.Length != 0)
     {
      result += delimiter;
     }
     result += s;
    }
    return result;
}

private static string CheckForEndInsideLoop_StringBuilder(string[] items, string delimiter)
{
    StringBuilder result = new StringBuilder();
    for (int i = 0; i < items.Length; i++)
    {
     result.Append(items[i]);
     if (i != items.Length - 1)
     {
      result.Append(delimiter);
     }
    }
    return result.ToString();
}

private static string RemoveFinalDelimiter_StringBuilder(string[] items, string delimiter)
{
    StringBuilder result = new StringBuilder();
    for (int i = 0; i < items.Length; i++)
    {
     result.Append(items[i]);
     result.Append(delimiter);
    }
    result.Length = result.Length - delimiter.Length;
    return result.ToString();
}

private static string StringJoin(string[] items, string delimiter)
{
    return String.Join(delimiter, items);
}

private static string Aggregate(string[] items, string delimiter)
{
    return items.Aggregate((c, s) => c + delimiter + s);
}

The results on my box are as follows:

00:00:00.0027745    True    StringJoin
00:00:24.5523967    True    Aggregate
00:00:47.8091632    True    CheckForEndInsideLoop_String
00:00:47.4682981    True    CheckForBeginningInsideLoop_String
00:00:23.7972864    True    RemoveFinalDelimiter_String
00:00:00.0076439    True    CheckForEndInsideLoop_StringBuilder
00:00:00.0052803    True    RemoveFinalDelimiter_StringBuilder

This means your best option, if you are only working with string arrays, is String.Join followed closely by the StringBuilder variants. Note that checking for the last item inside the loop makes a much larger difference when working with strings than it does when working with a StringBuilder. Performance for the String based implementations also improves quite a bit when the list of items to be delimited is small. I ran the same tests with numberOfItems set to 10 and received the following results:

00:00:00.0001788    True    StringJoin
00:00:00.0014983    True    Aggregate
00:00:00.0001666    True    CheckForEndInsideLoop_String
00:00:00.0002202    True    CheckForBeginningInsideLoop_String
00:00:00.0002061    True    RemoveFinalDelimiter_String
00:00:00.0002663    True    CheckForEndInsideLoop_StringBuilder
00:00:00.0002278    True    RemoveFinalDelimiter_StringBuilder

The next thing you might want to consider is reusability. If you wanted to build a string from a list of integers separated by a delimiter String.Join would only be an option after you ran .ToString() on each the integers and created a string array (because String.Join cannot act upon an IEnumerable<string>).

So, to conclude, you might consider using an extension method along the following lines to get a good combination of elegance, speed, and reusability:

public static string Join<T>([CanBeNull] this IEnumerable<T> items, [CanBeNull] string delimiter)
{
    StringBuilder result = new StringBuilder();
    if (items != null && items.Any())
    {
     delimiter = delimiter ?? "";
     foreach (T item in items)
     {
      result.Append(item);
      result.Append(delimiter);
     }
     result.Length = result.Length - delimiter.Length;
    }
    return result.ToString();
}

usage:

ns = sa.Join(" * ");
Handcraftsman
+1, An extension method is ideal for this. I posted a similar one, which also takes a converter function as a parameter, in response to another question: http://stackoverflow.com/questions/696850/opportunities-to-use-func-to-improve-code-readability/709875#709875
LukeH