Dotnet: .Net homepage should use foreach, not ForEach

[svick]

The .Net homepage contains a sample with this code:

```c#
var names = new List
{
"Ana",
"Felipe",
"Emillia"
};

names.ForEach(name =>
Console.WriteLine($"Hello {name}"));

The sample uses the somewhat controversial `List<T>.ForEach` method. I think that using this method is not a common style and might be confusing to beginners ("Why can't I use `.ForEach()` on this array?"). Because of that, I think the example should be changed to use `foreach` instead:

```c#
var names = new List<string>
{
    "Ana",
    "Felipe",
    "Emillia"
};

foreach (var name in names)
{
    Console.WriteLine($"Hello {name}");
}

This makes the code longer and it means it no longer showcases lambdas, but I think it's worth it to make it more idiomatic.

[richlander]

I agree.

@rowanmiller thoughts?

[rowanmiller]

This is probably one of those cases where folks will be split between the two options - with strong opinions. It's also important to keep in mind the purpose of that code snippet is to showcase C# and it targeted toward new developers (i.e. none of us that look at this repo 😄). I'm not saying that's a valid reason to be showing "bad code", but I think it does change how we pick what to show.

I am curious about List<T>.ForEach(...) being considered controversial, I don't have the background on that. Is there some place I can read up to learn more? I'd like to understand that, as it may change my mind 😄.

[svick]

@rowanmiller

This is probably one of those cases where folks will be split between the two options - with strong opinions.

Maybe. But considering that I think nobody will argue against foreach, while some will argue against ForEach, foreach sounds like the better choice to me.

It's also important to keep in mind the purpose of that code snippet is to showcase C# and it targeted toward new developers (i.e. none of us that look at this repo 😄). I'm not saying that's a valid reason to be showing "bad code", but I think it does change how we pick what to show.

Yes. That's why I said in the original post that ForEach might be confusing for beginners, since it's not universal and doesn't work on arrays or other collections.

And I think that you shouldn't show bad code (or "bad code") to beginners. If it's the first thing they see, they might keep using it for a long time. Because of that, I think that code targeted at beginners should be as idiomatic as possible.

If you want to showcase lambdas, maybe add a simple LINQ query (like a single Where) to the snippet?

I am curious about List<T>.ForEach(...) being considered controversial, I don't have the background on that. Is there some place I can read up to learn more? I'd like to understand that, as it may change my mind 😄.

I was mostly referring to this article by Eric Lippert. And again, there's the fact that no collection other than List<T> has ForEach. If ForEach was an uncontroversially considered a good thing, wouldn't it be supported more widely?

[rowanmiller]

We're only talking about bad code from a style perspective right? Just want to make sure I'm not missing something else.

[svick]

@rowanmiller Yeah, there is no correctness issue here.

[rowanmiller]

And just to be clear, I'm not saying you are wrong that we should change it... I just want to distill it down to what we are talking about.

If I could summarize, it seems like there are two potential reasons to change it.

• From a more subjective (but not un-important) style perspective, if foreach is used some folks will moderately prefer ForEach(...). But if ForEach(...) is used there will be a stronger negative reaction from folks who would prefer foreach.
• The fact that ForEach(...) is only available on List<T> may be confusing as folks start to use other collection types.

[svick]

@rowanmiller My main argument is that the snippet should be idiomatic C#, i.e. C# as it's commonly written. It's less about what people like or dislike and more about how people actually write their code. I haven't performed any analysis on this, but I do think foreach is more idiomatic than ForEach.

You got the second argument right.

[richlander]

I'm closing this since it doesn't seem to have a good path to resolution. Thanks for the feedback, @svick.

[svick]

@richlander What do you mean?

Are you saying that it's not clear that the code (which is now at https://www.microsoft.com/net/learn/languages) should be changed? One more argument in favor of changing it is that the VB example at the new page doesn't use the List<T>.ForEach method, it uses the For Each loop.

Or are you saying that you don't know how to change it? Surely someone at Microsoft has to be able to do it.