CODEWITHSUNDEEP
c#for-loopforeachtostring
hello
hello

Reputation: 1228

convert for loop to foreach with index

I have this ToString method used to write out objects. I'm trying to convert the for loop to a foreach loop. Without using LINQ.

Any pointers will be appreciated.

 public override string ToString()
 {
     StringBuilder output = new StringBuilder();
     output.AppendFormat("{0}", count);
     for (var index = 0; index < total; index++)
     {
         output.AppendFormat("{0}{1}{2} ", array[index], array[index].GetInfo,
                              string.Join(" ", array[index].Content(index)),
                            );
     }
     return output.ToString();
 }

Upvotes: 2

Views: 1167

Answers (3)

Enigmativity
Enigmativity

Reputation: 117027

Depending on what total and count are, you could refactor to something like this:

public override string ToString()
{
    return count.ToString()
        + String.Join(" ",
            array.Select((x, n) => $"{x}{x.GetInfo}{String.Join(" ", x.Content(n))}"));
}

String.Join can often perform better than StringBuilder so it's not a bad alternative.

There you go. No LINQ required with this extension:

public static class Ex
{
    public static IEnumerable<R> Select<T, R>(this IEnumerable<T> source, Func<T, int, R> projection)
    {
        int index = 0;
        foreach (var item in source)
        {
            yield return projection(item, index++);
        }
    }
}

Upvotes: 2

Nkosi
Nkosi

Reputation: 247018

Here is a refactoring based on your current code.

public override string ToString() {
    var output = new StringBuilder();
    output.AppendFormat("{0}", count);
    var index = 0;
    foreach (var item in array) {
        if (item!=null) {
            output.AppendFormat("{0}{1}{2} ", item, item.GetInfo,
                              string.Join(" ", item.Content(index++)),
                            );
        }
    }
    return output.ToString();
}

Using linq you could perform the same with a indexed Select without directly calling the foreach which is used under the hood 

public override string ToString() {
    var output = new StringBuilder();
    output.AppendFormat("{0}", count);
    array.Select((item, index) =>
        output.AppendFormat("{0}{1}{2} ", item, item.GetInfo,
                              string.Join(" ", item.Content(index)),
                            )
    );
    return output.ToString();
}

Upvotes: 3

Swagata Prateek
Swagata Prateek

Reputation: 1086

Technically, you can do something like this:

public override string ToString()
{
    StringBuilder output = new StringBuilder();
    output.Append(count);

    int index = 0;
    foreach (var item in array)
    {           
        output.Append($"{item}{item.GetInfo()}{string.Join(" ", item.Content(index))}");
        index++;
    }
    return output.ToString();
}

In any case, you're going to go for an int index anyway. Not so much of an help when it comes to moving it to foreach as per the performance goes.

You might try your own ForEach extension too if you really don't like writing incrementing indexes everywhere. 

public static class EnumerableExtensions
{
    public static void ForEachWithIndex<T>(this IEnumerable<T> sequence, Action<int, T> action)
    {
        // argument null checking omitted
        int i = 0;
        foreach (T item in sequence)
        {
            action(i, item);
            i++;
        }
    }
}

Then your method would be even simpler but you'd have one more class to manage:

public override string ToString()
{
    StringBuilder output = new StringBuilder();
    output.Append(count);
    array.ForEachWithIndex((index, item) => output.Append($"{item}{item.GetInfo()}{string.Join("", item.Content(index))}"));
    return output.ToString();
}

And for readability, if you really are just putting strings side by side:

public override string ToString()
{
    StringBuilder output = new StringBuilder();
    output.Append(count);
    array.ForEachWithIndex((index, item) => output.Append(
        string.Concat(
            item, 
            item.GetInfo(), 
            string.Join("", item.Content(index))
            )));
    return output.ToString();
}

As string.Concat would be more appreciated performance wise anyway. Opt for the others if you need formatting.

Upvotes: 2

Related Questions