CODEWITHSUNDEEP
c#optimizationextension-methodsmemory-management
Eric Liprandi
Eric Liprandi

Reputation: 5574

Good way to concatenate string representations of objects?

Ok,

We have a lot of where clauses in our code. We have just as many ways to generate a string to represent the in condition. I am trying to come up with a clean way as follows:

public static string Join<T>(this IEnumerable<T> items, string separator)
{
    var strings = from item in items select item.ToString();
    return string.Join(separator, strings.ToArray());
}

it can be used as follows:

var values = new []{1, 2, 3, 4, 5, 6};
values.StringJoin(",");
// result should be:
// "1,2,3,4,5,6"

So this is a nice extension method that does a very basic job. I know that simple code does not always turn into fast or efficient execution, but I am just curious as to what could I have missed with this simple code. Other members of our team are arguing that:

Any expert to chime in?

Regards,

Eric.

Upvotes: 2

Views: 248

Answers (5)

Steve Guidi
Steve Guidi

Reputation: 20200

For some reason, I thought that String.Join is implemented in terms of a StringBuilder class. But if it isn't, then the following is likely to perform better for large inputs since it doesn't recreate a String object for each join in the iteration.

public static string Join<T>(this IEnumerable<T> items, string separator)
{
    // TODO: check for null arguments.
    StringBuilder builder = new StringBuilder();
    foreach(T t in items)
    {
        builder.Append(t.ToString()).Append(separator);
    }

    builder.Length -= separator.Length;
    return builder.ToString();
}

EDIT: Here is an analysis of when it is appropriate to use StringBuilder and String.Join.

Upvotes: 3

kemiller2002
kemiller2002

Reputation: 115488

this would work also:

public static string Test(IEnumerable<T> items, string separator)
{
    var builder = new StringBuilder();
    bool appendSeperator = false;
    if(null != items)
    {
        foreach(var item in items)
        {
            if(appendSeperator)
            {
                builder.Append(separator)
            }

            builder.Append(item.ToString());

            appendSeperator = true;
        }
   }

   return builder.ToString();
}

Upvotes: 0

Daniel Br&#252;ckner
Daniel Br&#252;ckner

Reputation: 59655

You are missing null checks for the sequence and the items of the sequence. And yes, it is not the fastest and most memory efficient way. One would probably just enumerate the sequence and render the string representations of the items into a StringBuilder. But does this really matter? Are you experiencing performance problems? Do you need to optimize?

Upvotes: 0

Lee
Lee

Reputation: 144136

Regarding the first issue, you could add another 'formatter' parameter to control the conversion of each item into a string:

public static string Join<T>(this IEnumerable<T> items, string separator)
{
    return items.Join(separator, i => i.ToString());
}

public static string Join<T>(this IEnumerable<T> items, string separator, Func<T, string> formatter)
{
    return String.Join(separator, items.Select(i => formatter(i)).ToArray());
}

Regarding the second two issues, I wouldn't worry about it unless you later run into performance issues and find it to be a problem. It's unlikely to much of a bottleneck however...

Upvotes: 5

Nestor
Nestor

Reputation: 13990

Why don't you use StringBuilder, and iterate through the collection yourself, appending. Otherwise you are creating an array of strings (var strings) and then doing the Join.

Upvotes: 0

Related Questions