ForEach Extension

  • Thread starter Thread starter Shapper
  • Start date Start date
S

Shapper

Hello,

I created a ForEach extension as follows:

public static IEnumerable<T> ForEach<T>(this IEnumerable<T> source, Action<T> action) {
foreach (T item in source) {
action(item);
yield return item;
}
} // ForEach

However I need to use ToList() at the end to apply the actions.

So I created 2 other extensions and change the ForEach code:

IEnumerable<T> Apply<T>(this IEnumerable<T> source, Action<T> action) {
foreach (var item in source) {
action(item);
yield return item;
}
}

And then have another one as follows:

IEnumerable<T> Lock<T>(this IEnumerable<T> source) {
foreach (var item in source) {
; // Force to apply changes
}
return source;
}

This would be used as follows:

collection.Apply(....).Apply(....).Lock();

Does this make sense?

And the new ForEach extension would be:

public static void ForEach<T>(this IEnumerable<T> source, Action<T> action)
{
foreach (T item in source)
action(item);
}

However, using Apply and Lock I think this does not make sense to have.

What do you think?
 
So if I understood you correctly your suggestion is to simply use:

public static void ForEach<T>(this IEnumerable<T> source, Action<T> action)
{
foreach (T item in source)
action(item);
}

Without the yield so it executes immediately and use it once?

There is already a ForEach extension for IList.

So I though I should use yield for IEnumerable.

Does this make sense?

Thank You,
Miguel
 
I created a ForEach extension as follows:

public static IEnumerable<T> ForEach<T>(this IEnumerable<T> source, Action<T> action) {
foreach (T item in source) {
action(item);
yield return item;
}
} // ForEach
Click to expand...

This is bad code. ForEach implies that action es executed once for each
item in source. The implementation does something completely different.

In fact you implemented something like PreProcess, which preprocesses
each item before it is used in an iteration. Maybe there are use cases
for this code (e.g. updating the last used time stamps of cached items),
but it is definitely no replacement for a Loop.

However I need to use ToList() at the end to apply the actions.
Click to expand...

This would be a work around for the wrong code, no more, no less.

So I created 2 other extensions and change the ForEach code:

IEnumerable<T> Apply<T>(this IEnumerable<T> source, Action<T> action) {
foreach (var item in source) {
action(item);
yield return item;
}
}
Click to expand...

Still no intuitive naming.
And then have another one as follows:

IEnumerable<T> Lock<T>(this IEnumerable<T> source) {
foreach (var item in source) {
; // Force to apply changes
}
return source;
}

This would be used as follows:

collection.Apply(....).Apply(....).Lock();
Click to expand...

Are your applications by far to fast? Or what do you want to achieve?

If you need a dynamic number of actions to be taken for each item then
use an event. If the actions modify the item or if the sequence counts
for other reasons, use a list of delegates and execute them in a loop.

And the new ForEach extension would be:

public static void ForEach<T>(this IEnumerable<T> source, Action<T> action)
{
foreach (T item in source)
action(item);
}
Click to expand...

Hmm, while it looks neat to functional programmers, it is one of the
most efficient ways to eat CPU cycles in .NET.
Using this syntax prevents compiler optimizations.

However, using Apply and Lock I think this does not make sense to have.

What do you think?
Click to expand...

Exactly the same.


Marcel
 
I did some tests, read your answers and decided to not use the yield version.

I am going to implement and use only the following version:

public static void ForEach<T>(this IEnumerable<T> source, Action<T> action)
{
foreach (T item in source)
action(item);
}

As far as performance I will try to use a simple for loop most of the times.

In this case I was just using this on a simple string extension with an anonymous type:

new[] {
new { In = @"\s*&\s*", Out = String.Format(" {0} ", words[11]) },
new { In = @"\s*#", Out = String.Format(" {0} ", words[10]) },
new { In = @"\s*%\s*", Out = String.Format(" {0} ", words[12]) },
new { In = @"(\s*=\s*)", Out = String.Format(" {0} ", words[14]) },
new { In = @"\s*\+\s*", Out = String.Format(" {0} ", words[15]) },
new { In = @"\s*(\\|\/)\s*", Out = String.Format(" {0} ", words[13]) }
}.ForEach(x => phrase = Regex.Replace(phrase, x.In, x.Out));
 
In this case I was just using this on a simple string extension with an anonymous type:

new[] {
new { In = @"\s*&\s*", Out = String.Format(" {0} ", words[11]) },
new { In = @"\s*#", Out = String.Format(" {0} ", words[10]) },
new { In = @"\s*%\s*", Out = String.Format(" {0} ", words[12]) },
new { In = @"(\s*=\s*)", Out = String.Format(" {0} ", words[14]) },
new { In = @"\s*\+\s*", Out = String.Format(" {0} ", words[15]) },
new { In = @"\s*(\\|\/)\s*", Out = String.Format(" {0} ", words[13]) }
}.ForEach(x => phrase = Regex.Replace(phrase, x.In, x.Out));
Click to expand...

Feel free to do so, but what is wrong with

foreach (var x in new[] {
new { In = @"\s*&\s*", Out = String.Format(" {0} ", words[11]) },
new { In = @"\s*#", Out = String.Format(" {0} ", words[10]) },
new { In = @"\s*%\s*", Out = String.Format(" {0} ", words[12]) },
new { In = @"(\s*=\s*)", Out = String.Format(" {0} ", words[14]) },
new { In = @"\s*\+\s*", Out = String.Format(" {0} ", words[15]) },
new { In = @"\s*(\\|\/)\s*", Out = String.Format(" {0} ", words[13]) }
})
phrase = Regex.Replace(phrase, x.In, x.Out);

?


Marcel
 
Back
Top