{"id":1533,"date":"2009-10-07T17:50:00","date_gmt":"2009-10-07T17:50:00","guid":{"rendered":"https:\/\/blogs.msdn.microsoft.com\/vbteam\/2009\/10\/07\/getting-loopy-matt-gertz\/"},"modified":"2024-07-05T12:46:34","modified_gmt":"2024-07-05T19:46:34","slug":"getting-loopy-matt-gertz","status":"publish","type":"post","link":"https:\/\/devblogs.microsoft.com\/vbteam\/getting-loopy-matt-gertz\/","title":{"rendered":"Getting Loopy (Matt Gertz)"},"content":{"rendered":"

In my last post, I talked about the hidden costs that can occur whenever you call out to methods, particularly in loops.  <\/span>In looking at my examples, reader KG2V commented that another thing that folks need to be aware of is avoiding the assumption that the world (or, in this case, a list) is a static thing.  <\/span>It’s a good point and it deserves some attention, particularly given that different languages will react in different ways to change.<\/font><\/p>\n

To illustrate this point, let’s assume that we have a form with a ListBox on it, and that we also have a button on the form that, when clicked, runs the following VB code:<\/font><\/p>\n

    <\/span>‘ Bad — don’t do this<\/p>\n

<\/span><\/span><\/p>\n

    <\/span>Private<\/span> Sub<\/span> Button1_Click(ByVal<\/span> sender As<\/span> System.Object<\/span>, ByVal<\/span> e As<\/span> System.EventArgs<\/span>) Handles<\/span> Button1.Click<\/p>\n

<\/span><\/p>\n

        <\/span>For<\/span> i As<\/span> Integer<\/span> = 0 To<\/span> ListBox1.Items.Count – 1<\/p>\n

<\/span><\/p>\n

            <\/span>ListBox1.Items.RemoveAt(i)<\/p>\n

<\/span><\/p>\n

        <\/span>Next<\/span><\/p>\n

<\/span><\/p>\n

    <\/span>End<\/span> Sub<\/span><\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

Ostensibly, we want to remove all of the items from the ListBox.  <\/span>(Yes, normally we’d use Clear() to do that, but stay with me for a moment;  <\/span>I’ll switch to more “real” cases in a few paragraphs.)  <\/span>But if we run this code, we’ll get an exception.  <\/span>Why?  <\/span>Because whenever we remove an object, all of the indices of the objects after it will decrease by 1.  <\/span>Let’s assume that there are three objects in the list –  <\/span>A, B, and C – so that Count=3:<\/font><\/p>\n

·<\/font>         <\/span><\/span><\/span>When i = 0, we’ll remove the object at index 0, which is A.  <\/span>B now moves to index 0, and C now moves to index 1.<\/font><\/p>\n

·<\/font>         <\/span><\/span><\/span>When i = 1, we’ll remove the object at index 1, which is C.  <\/span>There is nothing after C, so nothing else changes.<\/font><\/p>\n

·<\/font>         <\/span><\/span><\/span>When i =2, we’ll remove the object at index 2, which is… nothing.  <\/span>There’s only one object left, which is B, and it’s at index 0.  <\/span>So, we get an out-of-bounds exception.<\/font><\/p>\n

In VB (as was pointed out to me in the comments section of my last point), the value of Count is only assessed once, right when the loop is encountered, so we will always loop through (in this case) three times.<\/font><\/p>\n

But is the lack of reevaluation of Count the only issue?  <\/span>Maybe not.  <\/span>Let’s try the same method in C#, in which Count is evaluated on every iteration:<\/font><\/p>\n

        <\/span>\/\/ Bad — don’t do this.<\/span><\/p>\n

<\/span><\/p>\n

        <\/span>private<\/span> void<\/span> button1_Click(object<\/span> sender, EventArgs<\/span> e)<\/p>\n

<\/span><\/p>\n

        <\/span>{<\/p>\n

<\/span><\/p>\n

            <\/span>int<\/span> i;<\/p>\n

<\/span><\/p>\n

            <\/span>for<\/span> (i = 0; i < listBox1.Items.Count; i++)<\/p>\n

<\/span><\/p>\n

            <\/span>{<\/p>\n

<\/span><\/p>\n

                <\/span>listBox1.Items.RemoveAt(i);<\/p>\n

<\/span><\/p>\n

            <\/span>}<\/p>\n

<\/span><\/p>\n

        <\/span>}<\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

·<\/font>         <\/span><\/span><\/span>When i = 0, we’ll remove the object at index 0, which is A.  <\/span>B now moves to index 0, and C now moves to index 1.<\/font><\/p>\n

o<\/font>   <\/span><\/span><\/span>Now, we’ll loop back to the top.  <\/span>The value of i becomes 1, and the value of Count becomes 2.  <\/span>The stop condition is not met, so we keep going.<\/font><\/p>\n

·<\/font>         <\/span><\/span><\/span>When i = 1, we’ll remove the object at index 1, which is C.  <\/span>There is nothing after C, so nothing else changes.<\/font><\/p>\n

o<\/font>   <\/span><\/span><\/span>Again, we loop back to the top.  <\/span>The value of i becomes 2, and the value of Count becomes 1.  <\/span>The stop condition is met, and we exit the loop.<\/font><\/p>\n

So, although no exception is thrown, the method also fails in C# — B is left behind.  <\/span>So, the problem isn’t whether or not Count is evaluated; it’s that the elements in the list are changing indices on each deletion.<\/font><\/p>\n

Sometimes doing things backwards is good<\/font><\/h2>\n

This is why, in the case where multiple removals need to be made on an indexed list, a smart programmer (all of whom are yawning right now as they read this) will work backwards.  <\/span>That is, they start with the largest index and work down towards zero.  <\/span>For example, imagine that we want to remove the 4th<\/sup>, 7th<\/sup>, and 9th<\/sup> element from a list.  <\/span>We don’t want to do it this way:<\/font><\/p>\n

        <\/span>‘ Bad — don’t do this<\/p>\n

<\/span><\/span><\/p>\n

ListBox1.Items.RemoveAt(4)<\/p>\n

<\/span><\/p>\n

ListBox1.Items.RemoveAt(7)<\/p>\n

<\/span><\/p>\n

ListBox1.Items.RemoveAt(9)<\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

Once we’ve removed the 4th<\/sup> item, then the element at 7 will have moved to 6 and the element at 9 will have moved to 8.  <\/span>After we’ve removed the new 7th<\/sup> item, the element at 8 will move to 7, and so on.  <\/span>We’d either be removing the wrong elements or we’d crash.  <\/span>It’s far better to do them in reverse order:<\/font><\/p>\n

ListBox1.Items.RemoveAt(9)<\/p>\n

<\/span><\/p>\n

ListBox1.Items.RemoveAt(7)<\/p>\n

<\/span><\/p>\n

ListBox1.Items.RemoveAt(4)<\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

None of the indices up front will be affected by changes that are occurring behind them in the list – their indices won’t change, and this will work fine.<\/font><\/p>\n

This issue comes up when you are (for example) deleting items from a listbox that supports multiselection.  <\/span>You basically have two ways to get the selected items in a listbox: you can use the SelectedIndices member function, which returns a collection of indices, or you can use SelectedItems, which returns a collection of selected items.  <\/span>Given either collection, there’s no way to say “remove all of these.” There is<\/b> a “Remove” command on the resulting collections, but if you use it, you’ll just be removing the item from *that* collection and not from the ListBox, which has its own collection of items – essentially, you’ll just deselect the items.  <\/span><\/font><\/font><\/p>\n

So, what do you do?  <\/span>At first, it looks like you have two options; you could try removing them from the master list based on item, or based on object.  <\/span>Let’s look at removal based on the item:<\/font><\/p>\n

        <\/span>‘ Bad — don’t do this<\/p>\n

<\/span><\/span><\/p>\n

        <\/span>For<\/span> Each<\/span> index As<\/span> Integer<\/span> In<\/span> ListBox1.SelectedIndices<\/p>\n

<\/span><\/p>\n

            <\/span>ListBox1.Items.RemoveAt(index)<\/p>\n

<\/span><\/p>\n

        <\/span>Next<\/span><\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

This won’t work.  <\/span>The selected indices are in numerical order (no matter in which order you selected them), so you’ll be sliding indices on each deletion and will subsequently delete the wrong things.  <\/span>You won’t crash if you overreach on an index, because the For…Each loop will check for Nothing when it does a MoveNext internally and will exit gracefully, but this is actually worse since it may take longer before you notice a problem!<\/font><\/p>\n

This won’t work either, and will potentially crash:<\/font><\/p>\n

        <\/span>‘ Bad — don’t do this<\/p>\n

<\/span><\/span><\/p>\n

        <\/span>For<\/span> index As<\/span> Integer<\/span> = 0 To<\/span> ListBox1.SelectedIndices.Count – 1<\/p>\n

<\/span><\/p>\n

            <\/span>ListBox1.Items.RemoveAt(ListBox1.SelectedIndices(index))<\/p>\n

<\/span><\/p>\n

        <\/span>Next<\/span><\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

Again, the indices for the downstream objects will decrease, you’ll delete the wrong things, and you’ll likely crash.  <\/span><\/font><\/font><\/p>\n

You can, however, iterate down instead, as long as you are confident that the selected indices are in numerical order (as they will be in this case):<\/font><\/p>\n

        <\/span>‘ This works:<\/span><\/p>\n

<\/span><\/p>\n

        <\/span>For<\/span> index As<\/span> Integer<\/span> = ListBox1.SelectedIndices.Count – 1 To<\/span> 0 Step<\/span> -1<\/p>\n

<\/span><\/p>\n

            <\/span>ListBox1.Items.RemoveAt(ListBox1.SelectedIndices(index))<\/p>\n

<\/span><\/p>\n

        <\/span>Next<\/span><\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

Having found something that works via iteration, let’s now consider the enumeration angle instead.  <\/span>In general, I prefer to enumerate lists whenever possible instead of iterating through them.  <\/span>This is not one of those times, though.  <\/span>Consider this code, which will fail:<\/font><\/p>\n

        <\/span>‘ Bad — don’t do this<\/p>\n

<\/span><\/span><\/p>\n

        <\/span>For<\/span> Each<\/span> item In<\/span> ListBox1.SelectedItems<\/p>\n

<\/span><\/p>\n

            <\/span>ListBox1.Items.Remove(item)<\/p>\n

<\/span><\/p>\n

        <\/span>Next<\/span><\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

“Well, c’mon, why doesn’t this<\/i> work?” I hear you cry.  <\/span>“I don’t care about indices in this case, I’m not even using them; I’m just removing specific items.”  <\/span>True, but if you run this code, you will indeed get an exception essentially stating that the collection has been modified and the list of selected items is no longer valid for enumeration.  <\/span>We can’t work around this by assigning the collection of selected items to another variable and enumerating on that, since that just creates a reference to the same collection.  <\/span>In order for it to work, you’ve have to get the references out of the collection itself so that the ListBox is no longer part of the equation on the removal loop’s assessment.<\/font><\/p>\n

        <\/span>‘ Really, this is just silly — don’t ever do this<\/span><\/p>\n

<\/span><\/p>\n

        <\/span>Dim<\/span> selItems As<\/span> New<\/span> Collection<\/span><\/p>\n

<\/span><\/p>\n

        <\/span>For<\/span> Each<\/span> item In<\/span> ListBox1.SelectedItems<\/p>\n

<\/span><\/p>\n

            <\/span>selItems.Add(item)<\/p>\n

<\/span><\/p>\n

        <\/span>Next<\/span><\/p>\n

<\/span><\/p>\n

        <\/span>For<\/span> Each<\/span> item In<\/span> selItems<\/p>\n

<\/span><\/p>\n

            <\/span>ListBox1.Items.Remove(item)<\/p>\n

<\/span><\/p>\n

        <\/span>Next<\/span><\/p>\n

<\/span><\/p>\n

<\/p>\n

 <\/p>\n

<\/span><\/p>\n

Functionally, this will work; however, you shouldn’t do it for two reasons:<\/font><\/p>\n

·<\/font>         <\/span><\/span><\/span>First of all, it’s stupid – you have to loop through twice.  <\/span>Occasionally, I do need to clone lists, like when creating an editable list from a read-only list – I’ve done that in a previous blogpost on this site, in fact.  <\/span> <\/span>But my motives are not so noble here; I’m using this for no other reason than allowing enumeration to work.<\/font><\/p>\n

·<\/font>         <\/span><\/span><\/span>Second, each call to RemoveItem has a hidden cost – namely, the list has got to go find that item.  <\/span>In this case, you can bet that there’s good hashing going on that doesn’t make that as painful as it might be, but do you really want to count on that, given the better numeric alternative above?<\/font><\/p>\n

No, just there’s really no good way to do removal by enumeration that I’ve ever found.  <\/span>And since each object is totally clueless about whether it itself is selected or not, you can’t even enumerate through the collection yourself and delete it if it’s marked as selected.<\/font><\/p>\n

Of course, I’ve just talked about deletions, since they are more interesting than additions due to the crash potential.  <\/span>However, additions can also mess you up in a loop due to similar index changes, and I’ve actually run into cases where a list may have both insertions and deletions done, so that I wasn’t even able to leverage the change in list size as a clue to figure out what had happened to the data.  <\/span>The bottom line is, whenever doing an operation within a loop, you need to understand whether or not that operation will collide with your loop conditions and assumptions.  <\/span>Generally, you control this in your code, but in multithreaded environments, where changes to an existing store of data might be made from a different thread “simultaneously” with your thread, you really need to understand how thread-safe your code is with respect to data changes.  <\/span>(But that is a story for another time.)<\/font><\/p>\n

‘Til next time,<\/font><\/p>\n

  <\/span>–Ma<\/font><\/font><\/p><\/p>\n","protected":false},"excerpt":{"rendered":"

In my last post, I talked about the hidden costs that can occur whenever you call out to methods, particularly in loops.  In looking at my examples, reader KG2V commented that another thing that folks need to be aware of is avoiding the assumption that the world (or, in this case, a list) is a […]<\/p>\n","protected":false},"author":258,"featured_media":8818,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[22,195],"tags":[101,165,166,167],"class_list":["post-1533","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-matt-gertz","category-visual-basic","tag-matt-gertz","tag-vb2005","tag-vb2008","tag-vb2010"],"acf":[],"blog_post_summary":"

In my last post, I talked about the hidden costs that can occur whenever you call out to methods, particularly in loops.  In looking at my examples, reader KG2V commented that another thing that folks need to be aware of is avoiding the assumption that the world (or, in this case, a list) is a […]<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/posts\/1533","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/users\/258"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/comments?post=1533"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/posts\/1533\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/media\/8818"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/media?parent=1533"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/categories?post=1533"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/vbteam\/wp-json\/wp\/v2\/tags?post=1533"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}