Czy tworzenie nowej listy w celu modyfikacji kolekcji w każdej pętli jest wadą projektową?

Niedawno natknąłem się na tę typową nieprawidłową operację Collection was modifiedw języku C # i choć w pełni ją rozumiem, wydaje się, że jest to tak częsty problem (Google, około 300 000 wyników!). Ale wydaje się również logiczną i prostą rzeczą modyfikowanie listy podczas jej przeglądania.

List<Book> myBooks = new List<Book>();

public void RemoveAllBooks(){
    foreach(Book book in myBooks){
         RemoveBook(book);
    }
}

RemoveBook(Book book){
    if(myBooks.Contains(book)){
         myBooks.Remove(book);
         if(OnBookEvent != null)
            OnBookEvent(this, new EventArgs("Removed"));
    }
}

Niektóre osoby utworzą kolejną listę, aby ją powtarzać, ale to tylko unika problemu. Jakie jest prawdziwe rozwiązanie lub jaki jest tutaj rzeczywisty problem projektowy? Wydaje się, że wszyscy chcemy to zrobić, ale czy wskazuje to na błąd projektowy?

Czy tworzenie nowej listy w celu modyfikacji kolekcji w każdej pętli jest wadą projektową?

Krótka odpowiedź: nie

Mówiąc prosto, wywołujesz niezdefiniowane zachowanie, gdy iterujesz kolekcję i modyfikujesz ją jednocześnie. Pomyśl o usunięcie z nextelementu w sekwencji. Co by się stało, gdyby MoveNext()się nazywało?

Moduł wyliczający pozostaje ważny, dopóki kolekcja pozostaje niezmieniona. Jeśli do kolekcji zostaną wprowadzone zmiany, takie jak dodawanie, modyfikowanie lub usuwanie elementów, moduł wyliczający zostanie nieodwracalnie unieważniony, a jego zachowanie będzie niezdefiniowane. Moduł wyliczający nie ma wyłącznego dostępu do kolekcji; dlatego wyliczanie za pomocą kolekcji nie jest wewnętrznie procedurą bezpieczną dla wątków.

Źródło: MSDN

Nawiasem mówiąc, możesz skrócić RemoveAllBooksto po prostureturn new List<Book>()

Aby usunąć książkę, zalecamy zwrócenie kolekcji z filtrem:

return books.Where(x => x.Author != "Bob").ToList();

Możliwa implementacja na półce wyglądałaby następująco:

public class Shelf
{
    List<Book> books=new List<Book> {
        new Book ("Paul"),
        new Book ("Peter")
    };

    public IEnumerable<Book> getAllBooks(){
        foreach(Book b in books){
            yield return b;
        }
    }

    public void RemovePetersBooks(){
        books= books.Where(x=>x.Author!="Peter").ToList();
    }

    public void EmptyShelf(){
        books = new List<Book> ();
    }

    public Shelf ()
    {
    }
}

public static void Main (string[] args)
{
    Shelf s = new Shelf ();
    foreach (Book b in s.getAllBooks()) {
        Console.WriteLine (b.Author);
    }
    s.RemovePetersBooks ();

    foreach (Book b in s.getAllBooks()) {
        Console.WriteLine (b.Author);
    }
    s.EmptyShelf ();
    foreach (Book b in s.getAllBooks()) {
        Console.WriteLine (b.Author);
    }
}

Utworzenie nowej listy w celu jej modyfikacji jest dobrym rozwiązaniem problemu, że istniejące iteratory listy nie mogą niezawodnie kontynuować po zmianie, chyba że wiedzą, jak zmieniła się lista.

Innym rozwiązaniem jest dokonanie modyfikacji za pomocą iteratora, a nie samego interfejsu listy. Działa to tylko wtedy, gdy może istnieć tylko jeden iterator - nie rozwiązuje problemu wielu wątków iterujących jednocześnie po liście, co (o ile dopuszczalny jest dostęp do nieaktualnych danych), powoduje utworzenie nowej listy.

Alternatywnym rozwiązaniem, które mogliby zastosować implementatorzy frameworku, jest śledzenie wszystkich iteratorów listy i powiadamianie ich o wystąpieniu zmian. Jednak narzuty związane z tym podejściem są wysokie - aby ogólnie działało z wieloma wątkami, wszystkie operacje iteratora musiałyby zablokować listę (aby upewnić się, że nie zmieni się w trakcie operacji), co spowoduje utworzenie listy operacje wolne. Pojawiłby się także nietrywialny narzut pamięci, a ponadto wymaga środowiska wykonawczego do obsługi miękkich odniesień, a IIRC pierwsza wersja CLR nie.

Z innej perspektywy, skopiowanie listy w celu jej modyfikacji pozwala użyć LINQ do określenia modyfikacji, co generalnie skutkuje wyraźniejszym kodem niż bezpośrednie iterowanie po liście, IMO.