Is There a Reason For C#'S Reuse of the Variable in a Foreach

Is there a reason for C#'s reuse of the variable in a foreach?

The compiler declares the variable in a way that makes it highly prone to an error that is often difficult to find and debug, while producing no perceivable benefits.

Your criticism is entirely justified.

I discuss this problem in detail here:

Closing over the loop variable considered harmful

Is there something you can do with foreach loops this way that you couldn't if they were compiled with an inner-scoped variable? or is this just an arbitrary choice that was made before anonymous methods and lambda expressions were available or common, and which hasn't been revised since then?

The latter. The C# 1.0 specification actually did not say whether the loop variable was inside or outside the loop body, as it made no observable difference. When closure semantics were introduced in C# 2.0, the choice was made to put the loop variable outside the loop, consistent with the "for" loop.

I think it is fair to say that all regret that decision. This is one of the worst "gotchas" in C#, and we are going to take the breaking change to fix it. In C# 5 the foreach loop variable will be logically inside the body of the loop, and therefore closures will get a fresh copy every time.

The for loop will not be changed, and the change will not be "back ported" to previous versions of C#. You should therefore continue to be careful when using this idiom.

Is it better coding practice to define variables outside a foreach even though more verbose?

The second form is no more wasteful - it's simply better.

There's no advantage to declaring the variables outside the loop, unless you want to maintain their values between iterations.

(Note that usually this makes no behavioural difference, but that's not true if the variables are being captured by a lambda expression or anonymous method.)

Reassign a variable a value in a foreach/for loop, and then use that value outside of the loop?

C# follows block scoping. Same named variable c and test2 are also present as member variables of your class. Using the same named variables inside your method creates new variable altogether and your member variables declared at class level get suppressed. So any change made to c and test2variables inside caesarEncryptText method will have no impact on member variables which are defined at class level.

class encryptionFunctionClass
{
char test2; // Poor Naming Conventions Ik.
char c;

public int caesarEncryptText(string inputText, int encryptionNumberShift)
{
textConvertedToArray.Add(inputText);
foreach (char c in inputText.ToString().ToCharArray())
{
//other code
//this test2 variable is NOT your member variable
char test2 = c;
//remaining code
}
}
}

To actually change the member variables instead you have two ways:

  1. Do not declare test2 variable locally inside your method. Then if you change test2 inside your method, it will change your member variable. This is what you should ideally be doing. Having same named variables at class level and at method level is dirty code.
  2. Access the member variable using this keyword inside your method if you want to change the member variable instead as shown below:

    foreach (char c in inputText.ToString().ToCharArray())
    {
    //other code
    //using this keyword will modify your member variable

    this.test2 = c;

    //remaining code
    }

Using Foreach Loop inside of Invoke

You just need an additional set of {}. Here your code is formatted so it is readable with the braces added.

public void MainLoop()
{
while (true)
{
this.Invoke(new MethodInvoker(() =>
{
foreach(GameObject ob in mygame.scenes[CurrentScene].objects)
{
//run code here
}
}
));
}
}

Editing dictionary values in a foreach loop

Setting a value in a dictionary updates its internal "version number" - which invalidates the iterator, and any iterator associated with the keys or values collection.

I do see your point, but at the same time it would be odd if the values collection could change mid-iteration - and for simplicity there's only one version number.

The normal way of fixing this sort of thing is to either copy the collection of keys beforehand and iterate over the copy, or iterate over the original collection but maintain a collection of changes which you'll apply after you've finished iterating.

For example:

Copying keys first

List<string> keys = new List<string>(colStates.Keys);
foreach(string key in keys)
{
double percent = colStates[key] / TotalCount;
if (percent < 0.05)
{
OtherCount += colStates[key];
colStates[key] = 0;
}
}

Or...

Creating a list of modifications

List<string> keysToNuke = new List<string>();
foreach(string key in colStates.Keys)
{
double percent = colStates[key] / TotalCount;
if (percent < 0.05)
{
OtherCount += colStates[key];
keysToNuke.Add(key);
}
}
foreach (string key in keysToNuke)
{
colStates[key] = 0;
}

ref foreach with List

It's because the Span<T> is equipped with an enumerator that has a ref Current property, and the List<T> isn't.

Span<T>.Enumerator.Current property:

public ref T Current { get; }

List<T>.Enumerator.Current property:

public T Current { get; }

If you want, you can get access to the internal storage of a List<T> with the CollectionsMarshal.AsSpan<T> method.


Update: Actually as @GuruStron pointed out in a comment, the C# compiler doesn't even use the Span<T>.Enumerator struct, and instead it translates a foreach loop to a fast while loop. For example the code below:

foreach (ref int item in span)
{
//...
}

...is translated to:

int i = 0;
while (i < span.Length)
{
ref int item = ref span[i];
//...
i++;
}

Foreach loop, determine which is the last iteration of the loop

If you just need to do something with the last element (as opposed to something different with the last element then using LINQ will help here:

Item last = Model.Results.Last();
// do something with last

If you need to do something different with the last element then you'd need something like:

Item last = Model.Results.Last();
foreach (Item result in Model.Results)
{
// do something with each item
if (result.Equals(last))
{
// do something different with the last item
}
else
{
// do something different with every item but the last
}
}

Though you'd probably need to write a custom comparer to ensure that you could tell that the item was the same as the item returned by Last().

This approach should be used with caution as Last may well have to iterate through the collection. While this might not be a problem for small collections, if it gets large it could have performance implications. It will also fail if the list contains duplicate items. In this cases something like this may be more appropriate:

int totalCount = result.Count();
for (int count = 0; count < totalCount; count++)
{
Item result = Model.Results[count];

// do something with each item
if ((count + 1) == totalCount)
{
// do something different with the last item
}
else
{
// do something different with every item but the last
}
}


Related Topics



Leave a reply



Submit