Why create a copy of the dictionary instead of using ToList()?

So I just finished this lecture on completing a transaction, and I googled the error I got before watching the rest of the video to see how y’all solved it. The top solution was simply this:

            foreach(InventoryItem item in transaction.Keys.ToList())
            {
                int quantity = transaction[item];
                for (int i = 0; i < quantity; i++)
                {
                    bool success = shopperInventory.AddToFirstEmptySlot(item, 1);
                    if(success)
                    {
                        AddToTransaction(item, -1);
                    }
                }
            }

I tried it out and it worked flawlessly, and had the benefit of being far less extra code to write. So, I’m wondering if there’s an issue with this solution that I’m not looking far enough ahead to be able to see?

ETA: To clarify, the code itself was not posted on the internet as a solution, but the idea of converting ToString before enumerating on a collection you need to edit was, and I tried it.

Perhaps it’s because ToList() is a Linq extension method. I can’t remember how much Linq Sam actually used, but it may be the reason. Or it could be that he wrote that before he started using Linq.

A lot of people frown upon the use of Linq in Unity because of performance, but it’s really just a matter of where you use it. While Linq is more than fast enough, it creates garbage and garbage collection is not fast. So, imo it’s fine to use Linq as long as it’s not being done on every frame. I can’t remember anything about the RPG course but this looks like it’s on the completion of a transaction and should therefore be fine.

I’m not seeing any issues with this. You’ve covered the major iteration over collection issue, that modifying the collection you’re iterating through will always fail (whether it’s in the foreach or the for loop).
Be aware that under the hood, Linq is making a complete copy of the Keys IEnumerable and then converting that copy into a list (that’s actually two copies). That’s not really an issue here, where we’re responding to an event. I personally used the .ToList() method as well in my personal projects. Just remember to avoid Linq like the plague within an Update Loop, if the expression will always be called every frame.

Privacy & Terms