Czy jeśli (items! = Null) jest zbędne przed foreach (pozycja T w pozycjach)?

105

Często napotykam kod podobny do następującego:

if ( items != null)
{
   foreach(T item in items)
   {
        //...
   }
}

Zasadniczo ifwarunek zapewnia, że foreachblok zostanie wykonany tylko wtedy, gdy itemsnie jest pusty. Zastanawiam się, czy ifstan jest naprawdę potrzebny, czy poradzę foreachsobie w przypadku items == null.

To znaczy, mogę po prostu napisać

foreach(T item in items)
{
    //...
}

bez martwienia się o itemsto, czy jest zerowy, czy nie? Czy ifstan jest zbędny? Czy to zależy od typu z itemslub może on Ttakże?

Nawaz
źródło
1
@ odpowiedź kjbartel użytkownika (w „ stackoverflow.com/a/32134295/401246 ” jest najlepszym rozwiązaniem, ponieważ nie: a) dotyczą degradacji wydajności (nawet jeśli nie null) uogólniając całą pętlę na wyświetlaczu LCD Enumerable(jak użycie ??byłoby ), b) wymagają dodania metody rozszerzenia do każdego projektu, lub c) wymagają unikania null IEnumerables (Pffft! Puh-LEAZE! SMH.) na początku (bo, nulloznacza N / A, podczas gdy pusta lista oznacza, że ​​jest stosowana, ale jest obecnie, cóż, pusty !, tj. pracownik może mieć prowizje, które nie dotyczą sprzedaży, lub są puste w przypadku sprzedaży, jeśli nie zarobili).
Tom

Odpowiedzi:

116

Nadal musisz sprawdzić, czy (items! = Null), w przeciwnym razie otrzymasz NullReferenceException. Możesz jednak zrobić coś takiego:

List<string> items = null;  
foreach (var item in items ?? new List<string>())
{
    item.Dump();
}

ale możesz sprawdzić jego wydajność. Więc nadal wolę mieć najpierw if (items! = Null).

Na podstawie sugestii Erica Lipperta zmieniłem kod na:

List<string> items = null;  
foreach (var item in items ?? Enumerable.Empty<string>())
{
    item.Dump();
}
Vlad Bezden
źródło
32
Ładny pomysł; preferowana byłaby pusta tablica, ponieważ zużywa mniej pamięci i powoduje mniejsze obciążenie pamięci. Enumerable.Empty <string> byłby jeszcze korzystniejszy, ponieważ buforuje pustą tablicę, którą generuje, i ponownie ją wykorzystuje.
Eric Lippert
5
Oczekuję, że drugi kod będzie wolniejszy. To degeneruje sekwencję do sekwencji, IEnumerable<T>która z kolei degraduje się do modułu wyliczającego do interfejsu, co spowalnia iterację. Mój test wykazał degradację 5-krotną dla iteracji w tablicy int.
CodesInChaos
11
@CodeInChaos: Czy zazwyczaj uważasz, że szybkość wyliczania pustej sekwencji jest wąskim gardłem wydajności w twoim programie?
Eric Lippert
14
Zmniejsza to nie tylko szybkość wyliczania pustej sekwencji, ale także całej sekwencji. A jeśli sekwencja jest wystarczająco długa, może to mieć znaczenie. Dla większości kodu powinniśmy wybrać kod idiomatyczny. Ale dwie alokacje, o których wspomniałeś, będą stanowić problem z wydajnością w jeszcze mniejszej liczbie przypadków.
CodesInChaos
15
@CodeInChaos: Ach, teraz rozumiem twój punkt widzenia. Gdy kompilator może wykryć, że „foreach” wykonuje iterację po List <T> lub tablicy, może zoptymalizować foreach, aby użyć modułów wyliczających typu wartości lub faktycznie wygenerować pętlę „for”. Kiedy zmuszony jest wyliczyć listę lub pustą sekwencję , musi wrócić do kodegenu „najniższego wspólnego mianownika”, który w niektórych przypadkach może być wolniejszy i powodować większe obciążenie pamięci. To subtelna, ale doskonała uwaga. Oczywiście morał tej historii jest - jak zawsze - jeśli masz problem z perfekcją, profiluj go, aby dowiedzieć się, co jest prawdziwym wąskim gardłem.
Eric Lippert
69

Używając C # 6, możesz użyć nowego operatora warunkowego o wartości null razem z List<T>.ForEach(Action<T>)(lub własną IEnumerable<T>.ForEachmetodą rozszerzającą).

List<string> items = null;
items?.ForEach(item =>
{
    // ...
});
kjbartel
źródło
Elegancka odpowiedź. Dzięki!
Steve
2
Jest to najlepsze rozwiązanie, ponieważ nie: a) nie pociąga za sobą pogorszenia wydajności (nawet jeśli nie null) uogólniania całej pętli na LCD Enumerable(jak przy użyciu ??), b) wymaga dodania metody rozszerzenia do każdego projektu lub c ) wymagają unikania null IEnumerables (Pffft! Puh-LEAZE! SMH.) na początku (bo, nulloznacza N / A, podczas gdy pusta lista oznacza, że ​​jest stosowana, ale obecnie jest, no cóż, pusta !, tj. Empl. może mieć Prowizje, które Nie dotyczy w przypadku pozycji niesprzedanych lub pustych w przypadku sprzedaży, jeśli nie zarobili).
Tom
6
@Tom: Zakłada się, że itemsjest to List<T>raczej myślenie niż byle jakie IEnumerable<T>. (Lub skorzystaj z niestandardowej metody rozszerzenia, o której powiedziałeś, że nie chcesz ...) Ponadto powiedziałbym, że naprawdę nie warto dodawać 11 komentarzy, a wszystkie zasadniczo mówią, że podoba ci się konkretna odpowiedź.
Jon Skeet
2
@Tom: Zdecydowanie odradzam ci to w przyszłości. Wyobraź sobie, że wszyscy, którzy nie zgodzili się z Twoim komentarzem, dodali swoje komentarze do wszystkich Twoich . (Wyobraź sobie, że napisałem tutaj swoją odpowiedź 11 razy.) To po prostu nie jest produktywne wykorzystanie przepełnienia stosu.
Jon Skeet
1
Zakładałbym również, że byłby hit w wykonaniu wezwania delegata w porównaniu ze standardem foreach. Szczególnie w przypadku listy, która moim zdaniem zostanie przekonwertowana na forpętlę.
kjbartel
37

Prawdziwym rozwiązaniem na wynos powinien być ciąg, który prawie nigdy nie powinien być zerowy . Po prostu uczyń niezmiennym we wszystkich swoich programach, że jeśli masz sekwencję, nigdy nie jest zerowa. Jest zawsze inicjowana jako pusta sekwencja lub inna oryginalna sekwencja.

Jeśli sekwencja nigdy nie jest zerowa, to oczywiście nie musisz jej sprawdzać.

Eric Lippert
źródło
2
A co jeśli uzyskasz sekwencję z usługi WCF? To może być zerowe, prawda?
Nawaz
4
@Nawaz: Gdybym miał usługę WCF, która zwróciłaby mi sekwencje o wartości null, które mają być pustymi sekwencjami, zgłosiłbym to jako błąd. To powiedziawszy: jeśli masz do czynienia ze źle sformułowanymi danymi wyjściowymi prawdopodobnie błędnych usług, to tak, musisz sobie z tym poradzić, sprawdzając, czy nie ma null.
Eric Lippert
7
Chyba że, oczywiście, puste i puste oznaczają zupełnie inne rzeczy. Czasami dotyczy to sekwencji.
konfigurator
@Nawaz Co powiesz na DataTable.Rows, która zwraca wartość null zamiast pustej kolekcji. Może to błąd?
Neil B
@ odpowiedź kjbartel użytkownika (w „ stackoverflow.com/a/32134295/401246 ” jest najlepszym rozwiązaniem, ponieważ nie: a) dotyczą degradacji wydajności (nawet jeśli nie null) uogólniając całą pętlę na wyświetlaczu LCD Enumerable(jak użycie ??byłoby ), b) wymagają dodania metody rozszerzenia do każdego projektu, lub c) wymagają unikania null IEnumerables (Pffft! Puh-LEAZE! SMH.) na początku (bo, nulloznacza N / A, podczas gdy pusta lista oznacza, że ​​jest stosowana, ale jest obecnie, cóż, pusty !, tj. pracownik może mieć prowizje, które nie dotyczą sprzedaży, lub są puste w przypadku sprzedaży, jeśli nie zarobili).
Tom
10

Właściwie na tym @Connect jest żądanie funkcji: http://connect.microsoft.com/VisualStudio/feedback/details/93497/foreach-should-check-for-null

Odpowiedź jest całkiem logiczna:

Myślę, że większość pętli foreach jest napisanych z zamiarem iteracji kolekcji niezerowej. Jeśli spróbujesz iterować przez null, powinieneś otrzymać wyjątek, abyś mógł naprawić swój kod.

Teoman Soygul
źródło
Myślę, że są w tym zalety i wady, więc zdecydowali się zachować to tak, jak zostało zaprojektowane w pierwszej kolejności. w końcu foreach to tylko cukier syntaktyczny. gdybyś wywołał items.GetEnumerator (), spowodowałoby to również awarię, gdyby itemy były puste, więc musisz najpierw to przetestować.
Marius Bancila
6

Zawsze możesz to przetestować z pustą listą ... ale to właśnie znalazłem na stronie internetowej msdn

foreach-statement:
    foreach   (   type   identifier   in   expression   )   embedded-statement 

Jeśli wyrażenie ma wartość null, zostanie zgłoszony System.NullReferenceException.

nbz
źródło
2

Nie jest to zbyteczne. W czasie wykonywania elementy zostaną rzutowane na IEnumerable i zostanie wywołana metoda GetEnumerator. Spowoduje to wyłuskanie elementów, które się nie powiodą

boca
źródło
1
1) Sekwencja niekoniecznie będzie rzutowana IEnumerablei 2) Decyzja o rzuceniu jest decyzją projektową. C # może łatwo wstawić nullsprawdzenie, czy programiści uznali to za dobry pomysł.
CodesInChaos
2

Możesz hermetyzować sprawdzanie wartości null w metodzie rozszerzenia i użyć wyrażenia lambda:

public static class EnumerableExtensions {
  public static void ForEach<T>(this IEnumerable<T> self, Action<T> action) {
    if (self != null) {
      foreach (var element in self) {
        action(element);
      }
    }
  }
}

Kod staje się:

items.ForEach(item => { 
  ...
});

Jeśli może być jeszcze bardziej zwięzłe, jeśli chcesz po prostu wywołać metodę, która pobiera element i zwraca void:

items.ForEach(MethodThatTakesAnItem);
Jordão
źródło
1

Potrzebujesz tego. Otrzymasz wyjątek, kiedyforeach dostęp do kontenera, aby skonfigurować iterację w inny sposób.

Pod okładkami foreachużywa interfejsu zaimplementowanego w klasie kolekcji do wykonania iteracji. Ogólny odpowiednik interfejsu jest tutaj .

Instrukcja foreach języka C # (dla każdego w Visual Basic) ukrywa złożoność modułów wyliczających. Dlatego zaleca się używanie foreach zamiast bezpośredniego manipulowania modułem wyliczającym.

Steve Townsend
źródło
1
Tak jak uwaga, technicznie nie używa interfejsu, używa pisania kaczego: blogs.msdn.com/b/kcwalina/archive/2007/07/18/ducknotation.aspx interfejsy zapewniają, że są tam odpowiednie metody i właściwości choć i pomóc w zrozumieniu intencji. jak również używać na zewnątrz dla wszystkich ...
ShuggyCoUk
0

Test jest konieczny, ponieważ jeśli kolekcja ma wartość null, foreach zgłosi NullReferenceException. Właściwie to całkiem proste, aby to wypróbować.

List<string> items = null;
foreach(var item in items)
{
   Console.WriteLine(item);
}
Marius Bancila
źródło
0

druga wyśle NullReferenceExceptionz wiadomościąObject reference not set to an instance of an object.

harryovers
źródło
0

Jak wspomniano tutaj , musisz sprawdzić, czy nie jest to wartość zerowa.

Nie używaj wyrażenia, którego wynikiem jest null.

Renatas M.
źródło
0

W C # 6 możesz napisać coś tak:

// some string from file or UI, i.e.:
// a) string s = "Hello, World!";
// b) string s = "";
// ...
var items = s?.Split(new char[] { ',', '!', ' ' }) ?? Enumerable.Empty<string>();  
foreach (var item in items)
{
    //..
}

Jest to w zasadzie rozwiązanie Vlada Bezdena, ale przy użyciu ?? expression, aby zawsze generować tablicę, która nie jest null i dlatego przetrwa foreach zamiast sprawdzania wewnątrz nawiasu foreach.

dr. rAI
źródło