Sign in to follow this  
thedustbustr

deleting list items while enumerating

Recommended Posts

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());

Share this post


Link to post
Share on other sites
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[i]))
{
listbox_windows.Items.Remove(i);
}
}


Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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...)

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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;
}
}
}
}

Share this post


Link to post
Share on other sites
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);
}

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this