7

Note: This already works fine, but I'm trying to understand Why it works this way, but not the other.

I have a WinForm (C#) with dynamically put images, like so: enter image description here

Now if you click the 'Napred' button, these images should be deleted (among other things), for which I originally used:

foreach(Control ctrl in Controls)
    if(ctrl is PictureBox) ctrl.Dispose();

or

for(int i = 0; i < Controls.Count; i++)
    if(Controls[i] is PictureBox) Controls[i].Dispose();

Now if I run this, I get:

enter image description here

But if I just change the for statement to send it backwards, it works?

for(int i = Controls.Count - 1; i >= 0; i--)
    if(Controls[i] is PictureBox) Controls[i].Dispose();

(I'm not going to upload another image, but it deletes all of the elements (I only get the buttons left in the end))

Can someone enlighten me why one works, but not the other?

EDIT: I'm using VS2015 community edition on Windows 10 if it's a debugging error(?)

Toza
  • 1,348
  • 2
  • 14
  • 35
  • If you have an array of 10 items and then delete Item1, Item2 will become new Item1 and you will have 9 items remaining. Standard method of disposing of array items is to step through it backwards. – LightBulb Dec 03 '15 at 09:24
  • Might be of interest for other approaches: http://stackoverflow.com/questions/7340757/c-sharp-list-removing-items-while-looping-iterating – shree.pat18 Dec 03 '15 at 09:25
  • Why don't you use Remove method? – Sergii Zhevzhyk Dec 03 '15 at 09:26
  • @SergiiZhevzhyk I did, pretty much the same result, but I've read elsewhere on SO that Remove() doesn't properly dispose of the element (memory-wise) and that this is a better solution. – Toza Dec 03 '15 at 09:28
  • @NemanjaT I totally agree that all resources should be properly disposed. I would probably remove the picture from the collection and then dispose it. – Sergii Zhevzhyk Dec 03 '15 at 09:31

1 Answers1

12

You're trying to change the list you're iterating over, which of course will change the indexes of this list so what was at index 1 is now at index 0.

By removing from the end of the array (I.e in your reverse), the previous indexes will always be the same.

Its also important to note as stated in Matthew Watson's comment:

Control.Dispose() is special and will remove the control from a parent container's Controls list.

This isn't default behavour by most Dispose methods so therefore you won't always find this behaviour when using Dispose

Sayse
  • 42,633
  • 14
  • 77
  • 146
  • 1
    I understand, that's why every 2nd element is being deleted. Thanks for clearing this up! It didn't cross my mind at all! – Toza Dec 03 '15 at 09:25
  • 2
    I think you should be clear about how he's changing the list over which he is iterating, because just from inspecting the code he is not removing items from the list directly. The answer is that `Control.Dispose()` is special and will remove the control from a parent container's `Controls` list. – Matthew Watson Dec 03 '15 at 09:26
  • @MatthewWatson - Very true, I was trying to keep this generic in reference to lists themselves – Sayse Dec 03 '15 at 09:27
  • But it would normally be fine to iterate in that way, disposing items in a list in order, since it wouldn't change the indices of the items in the list. It is extremely relevant that it is the `Controls` property that is being iterated. Just calling `Dispose()` on an item in a list does NOT generally change the list itself. – Matthew Watson Dec 03 '15 at 09:28
  • @MatthewWatson - I've included part of your comment in my answer now, I was trying to find a correct reference to this in msdn or developersource but wasn't able to. Hope this is ok! – Sayse Dec 03 '15 at 09:42