deleting list items while enumerating

Started by
7 comments, last by thedustbustr 16 years, 3 months ago
I have a listbox GUI element which is a list of open windows. If a window is closed, the list needs to be updated. This list is populated by a winapi function that enumerates open top-level windows. I call that function every 500ms to compare lists, updating if necessary. I'm using C#.
windowdetector.populate();
foreach (Window window in windowdetector.windows)
{
	if (!listbox_windows.Items.Contains(window))
	{
		listbox_windows.Items.Add(window);
	}
}
foreach (Window window in listbox_windows.Items)
{
	if (!windowdetector.windows.Contains(window))
	{
		listbox_windows.Items.Remove(window);
	}
}




However this causes a runtime exception because removing an element invalidates the iterator. How can I do this? It would be easier to clear the list and recopy it every time, but I can't lose my reference to the currently selected item.
//listbox_windows.Items.Clear();
//windowdetector.populate();			
//listbox_windows.Items.AddRange(windowdetector.windows.ToArray());

Advertisement
Try something along the lines of:

windowdetector.populate();foreach (Window window in windowdetector.windows){	if (!listbox_windows.Items.Contains(window))	{		listbox_windows.Items.Add(window);	}}for (int i = listbox_windows.Items.Count - 1; i >= 0; i--){	if (!windowdetector.windows.Contains(listbox_windows.Items))	{		listbox_windows.Items.Remove(i);	}}
i cant find any docs that guarantees that iterating backwards and deleting behind the iterator does not mess up the iterator (effectively 'sliding' the index undesirably). for all i know a conforming impl stores the list in reverse order, so your code could still break it. I did find a post on the MSDN forums talking about this and they suggested a similar workaround...

[edit]

FWIW your solution does seem to do what I asked in my OP. I'll assume its also defined.

New problem: I still lose my reference to the selected item. So backing up a level. How can I crop my list without invalidating ListBox.SelectedItem / ListBox.SelectedValue / ListBox.SelectedIndex, etc?
eew!!

First of all, how big are these mysterious 'Window' elements that you want to remove as you're enumerating? Removing elements from lists (in general) while you are enumerating them smells to me of dodginess. I understand exactly what you -want- to do, but I can't help but think you might want to eat up memory a little more and let the GC do 'removals' for you.

Idea is simple: instead of enumerating your collection and removing junk you don't want any more, construct a new list containing the items you -do- want to keep, excluding the stuff you -don't- want to keep. Once you're done, assign the old collection identifier to the new one and you're finished. Yes, it will chew up more memory, but you won't be fiddling with the collection while you're enumerating through it...

Really depends how often you call your function (500ms is not all that often...), how big your lists get, and how smart the GC is about cleaning up the old unassigned list elements.

Just a thought :)

~Shiny

[Edit]

Just saw your more recent post -- to maintain integrity of the view, if you go screwing around with the underlying collection of the listbox, you'll want to figure out what is currently displayed and store a copy of that while you do your add/removals, then manually reset it -- painful, sure, but predictable.
------------'C makes it easy to shoot yourself in the foot. C++ makes it harder, but when you do, it blows away your whole leg.' -Bjarne Stroustrup
Hi Shiny,

class Window{	private IntPtr _hwnd = IntPtr.Zero;	private string _title = "";	private string _module = "";

(and their public properties, two constructors, override ToString)

The list couldn't reasonably be more than 50 windows. Definitely not hundreds.

Your proposition was the first thing I tried, but it has an unacceptable consequence: it invalidates my references to the currently selected item (I'm deleting from a ListBox GUI widget's model...)
Everyone, thanks for your help. I have it working by storing the selected Window, clearing the listbox, and repopulating it, and then iterating through the new list and comparing hwnds to manually reselect the next.

One final minor problem: this fires every 500ms, and it noticibly flickers. In today's world of double buffering, this shouldn't ever happen. Do I need to enable double buffering somewhere?
Hi guys,

I've changed to a CheckedListBox, and now the flicker is intolerable.

Can anyone think of a better way?

private void RefreshWindowList(){	//http://www.gamedev.net/community/forums/topic.asp?topic_id=477581	Window selected = (Window)CheckedListBox_Windows.SelectedItem;	CheckedListBox_Windows.Items.Clear();	Program.windowdetector.populate();	CheckedListBox_Windows.Items.AddRange(Program.windowdetector.windows.ToArray());	//preserve the previous selection	if (selected == null)	{		//CheckedListBox_Windows.SelectedItem = windowdetector.windows[0]; //desktop	}	else	{		foreach (Window window in CheckedListBox_Windows.Items)		{			if (window.hwnd == selected.hwnd)			{				CheckedListBox_Windows.SelectedItem = window;				break;			}		}	}}
Events people, events.

Want you want to do is hook into the on_opening and on_closing events for the forms. That way the only time you add or remove is when you need to.

theTroll
Huh? I'm not aware of any events being fired when an arbitrary process's window opens. If you go to task manager and look closely, it's polling win32 every 500ms as well, to do the exact same thing I am.

Current iteration which I really thought was going to work, and suffers the same flicker problem, AND the selection doesn't get preserved:
            Program.windowdetector.populate();            IList StaleList = CheckedListBox_Windows.Items;            IList FreshList = Program.windowdetector.windows.ToArray();            IList JustOpenedWindows = new List<Window>();            IList ClosedWindows = new List<Window>();            //check for items to remove            foreach (Window window in StaleList)            {                if (FreshList.Contains(window)) continue;                ClosedWindows.Add(window);            }            //check for items to add            foreach (Window window in FreshList)            {                if (StaleList.Contains(window)) continue;                JustOpenedWindows.Add(window);            }            //remove all the closed windows            foreach (Window window in ClosedWindows)            {                CheckedListBox_Windows.Items.Remove(window);            }            //add all the freshly opened windows            foreach (Window window in JustOpenedWindows)            {                CheckedListBox_Windows.Items.Add(window);            }

This topic is closed to new replies.

Advertisement