Reassign object in foreach loop c#

Question

Not sure I understand why I can do this with a for loop and not a foreach loop?

This is the code that works. Looping through a BindingList Products, finding a match and then assigning that product at index i to the new product that's passed in.

 public static void UpdateProduct(int productToUpdateID, Product productToUpdate)
        {
            

            for (int i = 0; i < Products.Count; i++)
            {
                if (Products[i].ProductID == productToUpdateID)
                {
                    Products[i] = productToUpdate;
                }
            }
        }

If I try to do this with a foreach loop I get an error that I cannot assign to the iterator variable. What is the reasoning for this and is there a way to get around it or is using a for loop for this kind of problem the best solution?

This is essentially what I'm trying to do.

          public static void UpdateProduct(int productToUpdateID, Product productToUpdate)
        {

            foreach(Product product in Products)
            {
                if (product.ProductID == productToUpdateID)
                {
                    product = productToUpdate;
                }
            }
        }

I can do something like this and reassign all the properties explicitly but want to see if there is another way to do it.

            foreach(Product product in Products)
            {
                if (product.ProductID == productToUpdateID)
                {
                    product.Name = productToUpdate.Name;
                    
                }
            }

Thanks!

John Wu · Accepted Answer · 2022-12-30T23:37:08.577

2

The foreach construct is for when you want to do something with each item in the list. That does not seem to be what you are doing. You are modifying the list itself, by removing an item and replacing it.

Personally I would not use a loop at all, I'd just remove the old item and add the new one.

public static void UpdateProduct(int productToUpdateID, Product productToUpdate)
{
    Products.RemoveAll( x => x.ProductID == productToUpdateID );
    Products.Add( productToUpdate );
}

Or if you wish to preserve order:

public static void UpdateProduct(int productToUpdateID, Product productToUpdate)
{
    var index = Products.FindIndex( x => x.ProductID == productToUpdateID );
    Products[index] = productToUpdate;
}

edited Dec 30 '22 at 23:37

answered Dec 30 '22 at 21:43

John Wu

50,556
8
44
80

The second version doesn't only preserve order - it is more efficient too; removing from the middle involves shuffling everything, which an indexed swap avoids – Marc Gravell Dec 30 '22 at 22:49
I used a solution like the above where I was deleting the old and adding the new, but something about doing it that way seemed "dirty." Is there a convention for this or is this more efficient than updating the existing object? Thank you for the answer! – idkwhatimdoing Jan 01 '23 at 04:10
To me, the cleanest solution would be to use a dictionary instead of a list, and simply update it by key. This more clearly communicates what is going on and is more efficient for large lists, since the keys are held in a hashtable. I don't know of any special convention for remove+add. – John Wu Jan 01 '23 at 04:25

score 1 · Answer 2 · answered Dec 30 '22 at 22:47

The reasons have already been given, but as a minor detail: this is sometimes possible; there is an alternative syntax in recent C# that uses a ref-local for the iterator value:

foreach (ref [readonly] SomeType value in source)

which is only available for some scenarios - naked arrays, spans, or custom iterator types with a ref-return Current - and as long as the optional readonly modifier is not used, you can assign directly via the value variable, since this is a direct reference to the underlying source. The uses for this are rare and niche. If Products is a List<T>, you could combine this with CollectionMarshal.AsSpan(...) to achieve what you want, but frankly I'd consider that hacky (apart from other things, it would bypass the list's internal change protections). Basically: don't do this, but : it isn't entirely impossible.

score 1 · Answer 3 · answered Dec 31 '22 at 06:25

The foreach loop iterates over the elements of a collection, and the iteration variable is simply a reference to the current element in the collection.

The reason you cannot modify the iteration variable itself is that it is a read-only reference to the element in the collection. Modifying the iteration variable would not change the element in the collection; it would only change the reference.

Alternative ways are already mentioned in the above answers.

Serge · Answer 4 · 2022-12-30T23:49:27.210

Just for the record. IMHO the best way is to use a foreach loop with a modified code like this. It only makes one iteration

    int i=-1;
    foreach (var product in products )
    {
        i++;
        if (product.ProductID == productToUpdate.ProductID)
        {
            products[i]=productToUpdate;
            break;
        }
    }

But if you want to use linq for some reason, you can do it in one line

products = products.Select(x => x = x.ProductID == productToUpdate.ProductID?productToUpdate:x).ToList();

Reassign object in foreach loop c#

4 Answers4