1

I wanted to remove all elements in a collection and since I didn't see .RemoveAll or such I went with the code below.

int number = addressBook.Items.Count;
System.Collections.IEnumerator enumerator = ...
while (enumerator.MoveNext())
{
  target = enumerator.Current as Outlook.ContactItem;
  target.Delete();
}

However, I noticed that the number of remaining elements in the collection is roughly the half for each run of the program. My conclusion was that .Delete() skips to the next element by itself, meaning that .MoveNext() i nthe condition for the loop jumps to the elements after the next.

So I tried to reset the enumerator as follows.

int number = addressBook.Items.Count;
System.Collections.IEnumerator enumerator = ...
while (enumerator.MoveNext())
{
  target = enumerator.Current as Outlook.ContactItem;
  target.Delete();
  enumerator.Reset();
}

However, as I checked the .Count I saw that the number of elements remaining was still 1 after the last element being deleted and the enumerator being reset. And of course I got an exception thrown in my face.

What am I missing in this picture? I know it's not a bug because that would be reported and resolved eons ago...

Konrad Viltersten
  • 36,151
  • 76
  • 250
  • 438
  • Why don't you simply use a `foreach` loop??? – Jon Sep 07 '12 at 12:45
  • @Jon It's not a collection as you think about it. It's an Outlook story. I'm trying to remove contacts and it fights me. – Konrad Viltersten Sep 07 '12 at 12:50
  • @Tigran Line #1 in the code. `addressBook.Items.Count`. – Konrad Viltersten Sep 07 '12 at 12:50
  • 1
    @Jon - you can't modify a collection you're looping over with a `foreach` loop. – D Stanley Sep 07 '12 at 12:52
  • Maybe you can do something like `while (addressBook.Items.Count > 0) addressBook.Items[0].Delete();` – Kevin Gosse Sep 07 '12 at 12:54
  • @KooKiz It's an interop stuff in there. Intellisense gives my suggestion of `.Delete(int index)` and **then** claims that "there's no overloaded method taking 1 parameter". I have a screenshot to prove it! :) – Konrad Viltersten Sep 07 '12 at 13:00
  • @DStanley: But he's not modifying the collection, he's simply calling methods on the items. – Jon Sep 07 '12 at 13:02
  • 1
    On a side note, not sure if it's important but... http://msdn.microsoft.com/en-us/library/microsoft.office.interop.outlook._contactitem.delete.aspx "To delete all items in the Items collection of a folder, you must delete each item **starting with the last item** in the folder." – Kevin Gosse Sep 07 '12 at 13:07
  • @KooKiz It was **very** important. Put it together with the LINQ expression as an answer, please. – Konrad Viltersten Sep 07 '12 at 13:16

4 Answers4

3

IEnumerator can be used only as long as the underlying collection remains unchanged. As soon as it changes (e.g. by removing elements from it), the enumerator is irrecoverably invalidated.

You just cannot use one enumerator to browse a collection while modifying it, and Reset is not going to help with that. You just need to create a new enumerator every time. E.g. something like that might work:

while (addressBook.Items.Count > 0)
{
    foreach(Outlook.ContactItem target in ...)
    {
        target.Delete();
        break; // <-- !
    }
}
Mormegil
  • 7,955
  • 4
  • 42
  • 77
  • 1
    Or with linq: `while (addressBook.Items.Count > 0) addressBook.Items.OfType().First().Delete();` – Kevin Gosse Sep 07 '12 at 12:57
  • @KooKiz It seems that your LINQ-magic worked but I **strongly** dislike that I need to iterate through the collection myself... Put your code as an answer so people can grade it! And combine it with the link you've found. I was looking for it but must have missed it. Grrr to InterOp and Outlook at the moment. :) – Konrad Viltersten Sep 07 '12 at 13:10
1

See Marc Gravell's Answer.

Reset is redundant; so much so that it is a requirement in the language spec for iterator blocks to throw an exception on Reset. The correct thing to do is simply dispose and release the old iterator, and call GetEnumerator again. Or better: avoid having to read it twice, since not all data is repeatable.

Refer the following links:

Can't add/remove items from a collection while foreach is iterating over it
When IEnumerator.Reset() method is called?

using IEnumerator to removing items

Hope this help..

Community
  • 1
  • 1
Niranjan Singh
  • 18,017
  • 2
  • 42
  • 75
1

Two things.

  1. You should be able to delete all the items with an enumerator as long as you re-instantiate it after deleting each item. Using LINQ, it should look like:

    while (addressBook.Items.Count > 0)
    {
        addressBook.Items
            .OfType<Outlook.ContactItem>()
            .Last()
            .Delete();
    }
    
  2. According to MSDN, you actually have to delete the items starting with the last one: http://msdn.microsoft.com/en-us/library/microsoft.office.interop.outlook._contactitem.delete.aspx

Konrad Viltersten
  • 36,151
  • 76
  • 250
  • 438
Kevin Gosse
  • 38,392
  • 3
  • 78
  • 94
  • Great link but i question the coherence of the method. Why would I need to delete the last object first? I figure, suppose I only need to delete one contact, then, the next day, one more etc. I will end up in removing **all** the contacts **sans** following the method described **still** performing a valid operation. So why bother following **that** way? Am I logically correct or do I miss something? And it seems to me that `.First()` ought to be a teeny-weeny bit faster than `.Last()`. – Konrad Viltersten Sep 07 '12 at 13:35
  • @KonradViltersten I agree. What surprises me even more is that there's no recommendation on MSDN if you look at the documentation for Office 2007 (the link I gave is for Office 2010). So it's something recent, not just some legacy stuff. – Kevin Gosse Sep 07 '12 at 13:43
1

I expect that using the Enumerable.Reverse extension would a welcome improvement to while...Last().Delete() solution. Here's what that might look like:

foreach(var target in addressBook.Items
    .OfType<Outlook.ContactItem>()
    .Reverse())
{
    target.Delete();
}

I don't use Reverse that often; in fact, I don't recall ever using it in production code but I haven't had to inter-operate with Outlook either. The benefit here is that you're not repeatedly traversing the entirety of the list-remainder after each item deletion like the while...Last().Delete() method does. There may not be much (any?) difference in performance when the item lists are small, but if the traversal is expensive (and I imagine that it is since this likely comes down to COM iterop calls under the hood) or if the lists are large, then the performance difference could be significant. The only way to know for sure is to test/profile.

devgeezer
  • 4,159
  • 1
  • 20
  • 26
  • Can you `foreach` a collection that you're altering at the same time? Also, I think MS is wrong on this one and the removal of last-to-first element is a misunderstanding. Otherwise, it'd be invalid to remove the first element (only) and then, the next day, delete the new first element etc. until everything would be gone. Never the less - a cool usage of `Reverse`. I've never seen it in action either, actually. What puzzles me is that nobody checked the **question** as interesting, despite this many replies... – Konrad Viltersten Sep 08 '12 at 21:21
  • The iterator for `Reverse` reads everything in (it has to) before it iterates (unless they've done an optimization detecting `ICollection` enumeration) - so ordinarily I'd say you're right but I'm not sure here; regardless, all you would need to do to fix it is call `ToList` before iterating - then you'd be iterating over the list instance, not the original `IEnumerable` instance. – devgeezer Sep 08 '12 at 23:53
  • @KonradViltersten, I neglected to mention that the `GetEnumerator...while(enumerator.MoveNext())` in your sample is the logical equivalent of using a `foreach` loop (except that foreach will call Dispose on the enumerator when the loop exits.) – devgeezer Sep 09 '12 at 00:02
  • Hmmm... If the iterator needs to *read in* everything, I'm fairly certain that going `.First()` will be faster (or at least not slower under any circumstances). I'm glad this thread went into such a great discussion. I posted another question two days ago and **that** turned to be a real tumbleweed... :) – Konrad Viltersten Sep 09 '12 at 08:13
  • I think I get what you're saying - the implementation you settled on does a `First()` in a **while not empty** loop. My comments were aimed at deleting the items in last-to-first order - if that's not required then `First()` seems the way to go. Anyhow - I voted your question up as being interesting. :cheers: – devgeezer Sep 09 '12 at 21:11
  • Ah, great! I was worried that **MY** perception of interesting differed vastly from the rest of the community... :) – Konrad Viltersten Sep 10 '12 at 08:14