Celowe zgłaszanie wyjątków w połowach

10

Czy w typowym if...elsepakiecie z obsługą wyjątków coś w rodzaju poniższego przykładu jest zalecaną praktyką pozwalającą uniknąć powielania kodu?

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        throw new Exception();
    }
catch(Exception ex)
{
    return null;
}

zamiast...

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        return null;
    }
}
catch(Exception ex)
{
    return null;
}

Wiem, że nastąpił niewielki spadek wydajności, ale zastanawiam się, czy uważa się to za dopuszczalną praktykę. Obecnie robię to drugą metodą - szczególnie w przypadkach, w których muszę inaczej obsługiwać określone wyjątki - ale zastanawiałem się, czy pierwsza metoda jest odpowiednia dla prostych przypadków.

gajeNL
źródło
jeśli metoda jest wystarczająco mała, po prostu usuwam else i zwracam null poza blokiem trycatch, więc muszę zwracać null tylko raz.
Fabio Marcolini,

Odpowiedzi:

12

Microsoft odradza stosowanie obsługi wyjątków do kontroli przepływu.

Dostępny jest okrągły stół na ten temat.

Biorąc to pod uwagę, C # obsługuje to i przypuszczam, że zależy to od napotkanego warunku, czy wyjątek jest najbardziej odpowiednią odpowiedzią.

B2K
źródło
1
Uderza mnie to, że tego rodzaju rzeczy naprawdę bardzo starają się nie wykorzystywać wydarzeń.
radarbob
@radarbob: Jak powiązane są z tym wydarzenia?
@grovesNL - Zgłaszanie wyjątku w określonym punkcie w celu wywołania określonej metody w bloku catch? Kwaki jak dla mnie wydarzenie.
radarbob
@radarbob: To nie jest wydarzenie. Istnieje wiele przykładowych przypadków użycia, w których można by to wykorzystać, jak omówiono w linku okrągłego stołu w odpowiedzi.
1
@radarbob Trochę wyjaśnienia, wyjątki zaprojektowano jako sposób zasygnalizowania dzwoniącemu, że zaszło coś, czego wywoływana metoda nie jest w stanie obsłużyć. Należy jednak wysłuchać zdarzenia. Wyjątkiem jest wymuszone przerwanie normalnego przebiegu programu. Nieprzechwycony wyjątek spowoduje zakończenie działania całej aplikacji.
6

Trafienie w wydajność jest prawdopodobnie znikome, jak wyjaśniono w tej odpowiedzi .

Przejdźmy więc do wniosku, że wydajność nie stanowi problemu. Rzucasz System.Exception, aby przenieść egzekucję do catchklauzuli . BadControlFlowThatShouldBeRewrittenExceptionJednak rzucanie byłoby prawdopodobnie przesadą.

Rozwalmy to. Mamy:

  • Metoda GetDataFromServer(nazwy metod powinny być PascalCase w języku C #), które mogą wyrzucić wyjątek lub zwrócić a bool.
  • Jeśli wynik był true, uruchom ProcessData.
  • Wróć nullinaczej.

Wygląda na to, że metoda, w której napisany jest ten kod, po prostu robi zbyt wiele rzeczy. GetDataFromServerzwracanie boolwygląda jak błąd projektowy, oczekiwałbym, że ta metoda zwróci dane otrzymywane z serwera , niektóre IEnumerable<SomeType>zawierające 0 lub więcej elementów - tj. szczęśliwa ścieżka zwraca n elementów, gdzie n> 0 , niezbyt szczęśliwy ścieżka zwraca 0 elementów, a niezadowolona ścieżka wysadza się w powietrze z nieobsługiwanym wyjątkiem, cokolwiek to jest.

To bardzo zmienia wygląd metody - znowu trudno jest stwierdzić, czy ma to sens, ponieważ oryginalny post ma tylko jeden punkt wyjścia (a zatem nie skompiluje się, ponieważ nie wszystkie ścieżki kodu zwracają wartość ), więc to tylko dzikie przypuszczenie:

try
{
    var result = GetDataFromServer();
    return ProcessData(result);
}
catch
{
    return null;
}

Tutaj możesz zobaczyć ProcessDatai zobaczyć, że iteruje iterację result, i zwraca, nulljeśli nie ma żadnego elementu w IEnumerable.

Dlaczego metoda się zwraca null? Serwer nie działa? Czy w zapytaniu jest błąd? Ciąg połączenia używa niewłaściwych danych logowania? Ilekroć GetDataFromServerwybuchnie wyjątek, którego się nie spodziewasz, połykasz go, wsuwasz pod dywan i zwracasz nullwartość. W tym przypadku zalecam wychwycenie określonych wyjątków i zapisanie wszystkiego; debugowanie będzie w ten sposób znacznie łatwiejsze.

Z ogólną catchklauzulą, która nie wychwytuje wyjątku, dość trudno jest zdiagnozować cokolwiek. Zamiast tego zrobiłbym to minimalnie:

catch(Exception e)
{
    return null;
}

Teraz możesz przynajmniej złamać i sprawdzić, eczy coś pójdzie nie tak.


TL; DR : Nie, rzucanie i wyławianie wyjątków dla kontroli przepływu nie jest dobrym pomysłem.

Mathieu Guindon
źródło
Ta odpowiedź pokazuje dokładnie, dlaczego starałem się zachować ogólny kod: nie chciałem wymieniać każdego wyjątku, który faktycznie robię w moim kodzie; Nie chciałem wymieniać rzeczywistych nazw metod; Nie chciałem wyświetlać deklaracji metody; Nie chciałem sugestii dotyczących składni. Miałem jedno pytanie, czy rzucanie wyjątków w zakresie kontroli przepływu było w porządku, na co natychmiast odpowiedziała B2K. Z przyjemnością omówię to na stronie meta.
3
Wygląda na to, że to powinno być pytanie dla programistów. sprawdzamy kod, a nie pomysły.
Malachi
2

w twojej pierwszej odpowiedzi jest hit wydajności, który nie musi tam być.

try
{
    if (GetDataFromServer())
    {
        return ProcessData();
    }
    else
    {
        throw new Exception();
    }
catch(Exception ex)
{
    return null;
}

po wyjściu z instrukcji if, aby wejść do instrukcji Catch, gdy nie trzeba zmuszać kodu do zmiany kierunku, że tak powiem.

jeśli chcesz to return null; zrobić w instrukcji else, a nie w połowie, który jest złapany po wyrzuceniu z instrukcji else.

Prawdopodobnie nie dotyczy twojego prawdziwego kodu, ale ma zastosowanie do kodu ogólnego, który podałeś.

Normy mówią, że nie powinieneś tego robić.

Standardy mówią, że powinieneś to zrobić w ten sposób (ponownie na podstawie kodu ogólnego podanego w OP)

if (GetDataFromServer())
{
    return ProcessData();
}
else
{
    Return null
}

a ponieważ nie masz żadnych wyjątków, które łapiesz, nie powinieneś nawet próbować tutaj łapać.

chcesz zobaczyć wyjątki, gdy wystąpią, aby można było rozwiązać problem, który tworzy wyjątek.

Malachiasz
źródło
1

Dlaczego nie o wiele prostsze:

if (!GetDataFromServer()) return null;
ProcessData();

Jeśli moduł obsługi wyjątków będzie istniał, powinien być w ProcessData ()

Loren Pechtel
źródło
Dlaczego nie miałbym chcieć przekazywać wyjątków ProcessData()na najwyższy poziom?
gajeNL
@grovesNL Nie zrobiono nic użytecznego z wyjątkiem tutaj.
Loren Pechtel
1
Jak to? Jeśli ProcessData()zgłasza wyjątek, teraz nie jest to obsługiwane. Chcę to zrobić return nullna tym poziomie, jeśli ProcessData()zgłosi wyjątek, nie zmieniając ProcessData()się.
gajeNL