0

I have been having an issue lately with a Foreach statement and it throwing an Collection was modified; enumeration operation may not execute exception when changing a variable that shouldn't impact the items being enumerated upon, but it still does. Here is my code so I can continue my explanation:

    private static CEconTradeOffer RemoveUndesirables(CEconTradeOffer offer)
    {
        try
        {
            CEconTradeOffer returned = offer;
            foreach (CEconAsset cEconAsset in offer.ItemsToReceive)
            {
                string marketHashName = cEconAsset.GetMarketHashName(_config.ApiKey);
                if (marketHashName.ToLower().Contains("case") ||
                    marketHashName.Contains("gut") ||
                    marketHashName.Contains("falchion") ||
                    marketHashName.Contains("bayonet") ||
                    marketHashName.Contains("huntsman") ||
                    marketHashName.Contains("karambit") ||
                    marketHashName.Contains("butterfly"))
                {
                    //somehow changes both "offer" and "returned" at once.
                    returned.ItemsToReceive.Remove(cEconAsset);
                    continue;
                }

                MarketValue value = MarketHandler.GetPriceOverview(Convert.ToUInt32(cEconAsset.AppId),
                    marketHashName);
                if (!value.Success || int.Parse(value.Volume, NumberStyles.AllowThousands) <= 20)
                    returned.ItemsToReceive.Remove(cEconAsset);
            }
            return returned;
        }
        catch (Exception e)
        {
            Write.Exception(e);
            return null;
        }
    }

This function was designed to do exactly what it says; remove undesired items from a trade offer. As you can see, I set CEconTradeOffer returned equal to the passed argument of the same type named offer. Strangely enough, whenever I change something inside of returned it causes the foreach statement to break even though I technically shouldn't be impacting offer.ItemsToReceive in any way. returned is what should be modified. When I use the debugger, I notice that BOTH returned and offer get changed at the line returned.ItemsToReceive.Remove(cEconAsset);. Through my previous experience with C# and browsing related problems, this should not be happening since I am creating a new variable that should be separate from offer. I have tried include setting returned equal to a new CEconTradeOffer() and then setting it equal to offer but to no avail. In my research of this problem, I can only seem to find problems where people fail to create a new variable and modifying that in the foreach statement rather than the enumerated value.

Is there something blatantly obvious I am missing? I do not quite understand why I am getting this particular issue when I create a separate variable to change inside the foreach statement.

I am not using more than one thread, so it can't be impacted outside of its own thread of execution.

Thanks in advance.

Chris Altig
  • 680
  • 3
  • 8
  • 22
  • AlexD points out the problem in his answer below: `returned` IS `offer`, so the remove method modifies the collection and you get the error--don't modify a collection during enumeration. – D. Ben Knoble Sep 28 '15 at 00:40
  • 1
    Useful reading - [Shallow vs. Deep copy](http://stackoverflow.com/questions/184710/what-is-the-difference-between-a-deep-copy-and-a-shallow-copy) – Alexei Levenkov Sep 28 '15 at 01:11

1 Answers1

4

whenever I change something inside of returned it causes the foreach statement to break even though I technically shouldn't be impacting offer.ItemsToReceive in any way.

It seems that CEconTradeOffer is a reference type, so then you assign

CEconTradeOffer returned = offer;

you get another reference to exactly the same thing.

So when the items are removed using returned.ItemsToReceive.Remove(cEconAsset) they are removed from the same collection, just via another reference.

AlexD
  • 32,156
  • 3
  • 71
  • 65
  • I spotted it too but you got there first :P – D. Ben Knoble Sep 28 '15 at 00:40
  • Ah, okay. Is there a way to rectify this? – Chris Altig Sep 28 '15 at 02:50
  • 1
    @ChrisAltig I did not check the logic. But if you want to modify a collection while enumerating it, you may use regular for loop. (If you delete items it may be easier to run from right to left, so removing items does not impact the rest.) – AlexD Sep 28 '15 at 04:54
  • @AlexD I believe you are correct. My only other option is a deep copy which would slow down the program (not by much, but still). I find it strange that it is used as a reference rather than entirely new variable. – Chris Altig Sep 28 '15 at 21:55