[JackOfAllTrades]

Monster.Clear();

Well, you are clearing the List within the foreach loop, which would certainly be an issue. You're pulling out the rug from under your feet.

[Curtis Rutland]

.Clear() deletes every entry in a list. Why would you want to try to clear the list on your way through a foreach loop?

Anyway, you're not allowed to modify the collection that you're doing a "foreach" on. There's no reason to remove it, foreach will not go to an element twice.

[Zunera]

Hi,
you just put the end of you foreach-loop at the wrong position - it should end before making the output of how many exp and gold the player gets.
The Monster.Clear() and the whole piece of code is fine if you end the loop after line 11.

This post has been edited by Zunera: 14 December 2010 - 11:41 PM

[tlhIn`toq]

Personally I wouldn't use a ForEach in this case. I'd use a for, starting at the end of the list and working toward the start. That way you can delete as you co and not affect the index

int Max = MyMonstersList.Count-1;

for (int Index = Max; Index > -1; Index--)
{
   monster ThisMonster = MyMonsterList[Index];
   MyHero.Gold += ThisMonster.Gold;
   // Distribute other Monster properties
   ThisMonster.Dispose(); // This is the laster monster lets say #14. We decrement the loop.  So the next one will be #13
}

Do you see how there is no interference here by working from the end?

[Zunera]

tlhIn, on 15 December 2010 - 07:49 AM, said:

Personally I wouldn't use a ForEach in this case. I'd use a for, starting at the end of the list and working toward the start. That way you can delete as you co and not affect the index

int Max = MyMonstersList.Count-1;

for (int Index = Max; Index > -1; Index--)
{
   monster ThisMonster = MyMonsterList[Index];
   MyHero.Gold += ThisMonster.Gold;
   // Distribute other Monster properties
   ThisMonster.Dispose(); // This is the laster monster lets say #14. We decrement the loop.  So the next one will be #13
}

Do you see how there is no interference here by working from the end?

GuysGuysGuys, whats going on in this forum?? Sorry, but how can somebody be an expert and make advices like this??? This is working as well without any differences:

for (int Index = 0; Index < MyMonstersList.Count; Index++)
{
   monster ThisMonster = MyMonsterList[Index];
   MyHero.Gold += ThisMonster.Gold;
   ThisMonster.Dispose();
}

You also have to mention that the Dispose()-method is by default not a member of an object - its available for sure if a class implements IDisposable, which is mostly not necessary.

And even more, this code is working as well (if monster-class has a Dispose()-method):

foreach (monster ThisMonster in MyMonsterList)
{
   MyHero.Gold += ThisMonster.Gold;
   ThisMonster.Dispose();
}

At the end it doesn't make no difference in this case if you loop from the end to the beginning or vice versa... And personally I prefer the Foreach-Loop cause its more clear for me at the first look and its not so "risky" e.g. by playing around with Index within the loop.

[JackOfAllTrades]

Zunera, in my reply it was a case of jumping on the obvious problem at hand, rather than seeing the bigger picture.

I guess using Dispose() doesn't disrupt the internal iterator of the collection?

[Zunera]

JackOfAllTrades, on 15 December 2010 - 11:42 AM, said:

Zunera, in my reply it was a case of jumping on the obvious problem at hand, rather than seeing the bigger picture.

I guess using Dispose() doesn't disrupt the internal iterator of the collection?

Yeah sorry, i might have been a bit overreaction on this thread but your and insertAlias answers were describing the effect and not the cause - removing Monster.Clear() or putting it outside the loop would suppress the error but wouldn't solve the real problem, therefore it seemed very misleading to me.

And yes, in general the call of Dispose() has no effect on an iteration over the collection the object is in - but of course it always depends on the stuff that happens in the method if you are defining a custom Dispose() method.

[tlhIn`toq]

Zunera, on 15 December 2010 - 01:53 AM, said:

GuysGuysGuys, whats going on in this forum?? Sorry, but how can somebody be an expert and make advices like this???

I realize with my pidly 594 reputation points that I'm no where near as qualified as you with your 10 points and 8 days of D.I.C. membership... but may I point out something?

Zunera, on 15 December 2010 - 01:53 AM, said:

This is working as well without any differences:

for (int Index = 0; Index < MyMonstersList.Count; Index++)
{
   monster ThisMonster = MyMonsterList[Index];
   MyHero.Gold += ThisMonster.Gold;
   ThisMonster.Dispose();
}

If you loop from the start to the end - lets say 0 to 100, and dispose of the items as you move forward, yet still increment the counter... You skip every other entree. You may not notice this behavior during initial testing if you only have a couple Monsters in your list, or you fire it up, run a fast test then kill it. Its easy to overlook if you don't put in breakpoints and watch the variables pallet as the loop executes. If your list is Monsters: Bob, John, Mary, Ringo, Apollo, Yogi you have to be paying close attention to see you are only working on Monsters Bob, Mary, Apollo and somehow skipping Joh, Ringo and Yogi.

• On the first pass the index is 0
  
• Then you dispose of 0, so element 1 becomes element 0, element 2 becomes 1
  
• Then your loop increments the counter to 1 - but the current 1 is the old 2.
  
• You have skipped the old 1 because it is now in position 0 and your counter has jumped over that.

This is the heart of the problem with using the ForEach loop construct and disposing of the lowest number index as you do so. Essentially a ForEach and For incrementing by +1 are the same thing. Which was the OP's original problem.

Dan064 said:

Then it seems to blow up in the Foreach loop, because the number has been altered

[Zunera]

tlhIn, on 15 December 2010 - 06:39 PM, said:

I realize with my pidly 594 reputation points that I'm no where near as qualified as you with your 10 points and 8 days of D.I.C. membership... but may I point out something?

You may and you did and i would give you a -1 but i let be this time - even if I'm very wondered that some admin just gave you a good reputation for your comment cause its totally wrong - like your last one.

Disposing an object doesn't remove it from a collection - there might be some special cases if the object may only exist as part of a collection, but for default List-objects you can call Dispose() without any effect on the list cause the object itself doesn't 'know' anything about a collection it might be part of. Try this:

            List<Form> formList = new List<Form>();
            formList.Add(new Form());
            formList.Add(new Form());
            foreach (Form form in formList)
            {
                form.Dispose();
            }

I just used Form-object as a well known disposable object.

tlhIn said:

This is the heart of the problem with using the ForEach loop construct and disposing of the lowest number index as you do so. Essentially a ForEach and For incrementing by +1 are the same thing. Which was the OP's original problem.

Dan064 said:

Then it seems to blow up in the Foreach loop, because the number has been altered

JackOfAllTrades and insertAlias already pointed out what the reason for the exception is - Monster.Clear() within the loop. And I already pointed out what the cause and the real problem of the given piece of code is.

This post has been edited by Zunera: 15 December 2010 - 01:22 PM

[JackOfAllTrades]

Logically that seems to make complete sense, where you're operating on the object itself rather than the collection.