Single Exit Point

Najprościej definiując problem: Czy mamy tylko jeden return z metody, czy też mamy ich kilka w różnych miejscach.

Mówię też o metodach, które nie zwracają wartości. Metodę void też można skończyć wcześniej:

void FooBar(int[] array)
{
    if (array == null || array.Length == 0)
        return;

    // proper processing
}

Najpierw usłyszałem, że to dobre podejście

Pamiętam z podstaw programowania w jakimś C++, że flow metod powinien mieć Single Exit Point. Trzeba to robić dla lepszej czytelności kodu, „wyskawiwanie” w dziwnych miejscach nie jest dobre. Więc coś co wygląda tak (już używając naszego C#):

int Foo(int[] array)
{
    for (int i = 0; i < array.Length; i++)
    {
        if (array[i] == 2)
        {
            return i * i;
        }
    }

    return 0;
}

powinno wyglądać tak:

int Bar(int[] array)
{
    int result = 0;
    for (int i = 0; i < array.Length; i++)
    {
        if (array[i] == 2)
        {
            result = i * i;
            i = array.Length; // or break;
        }
    }

    return result;
}

Prosty kod – uciekam od razu

Trochę czasu minęło i na co dzień piszę kod, który szybko „ucieka” z metod. Nie widzę w tym nic złego, np:

if (item == null)
    return null;
 
if (item.ColorInHex == null)
    return null;
 
var colorName = GetColorName(item.ColorInHex);
return colorName;

Trzeba jednak zaznaczyć, że działa to „raczej” głównie dla prostych zwracanych wartości jak np. null lub czysty return;.

Przypadek bardziej skomplikowanego kodu

Ostatnio trafiłem na przypadek gdy jednak nie chciałem mieć wielu returnów. Pomagałem koledze zrefaktorować pewien kawałek kodu. Problem był taki, że funkcja była już przerośnięta, słabo czytelna i ciężko było dodać jej kolejną funkcjonalność (dlatego sie refaktoryzuje nawet gdy nikt nie zmusza).

W kilku miejscach były zwracane pozornie różne wartości, zagnieżdżone ify, ktoś mógłby to nazwać spaghetti code. Mimo to istniał jeden wspólny algorytm, który można było zastosować. Niestety, żeby go zauważyć trzeba było to przerobić właśnie zgodnie z zasadą Single Exit Point.

Obydwa podejścia dobre

Nie ma generalnej zasady co lepsze. W jednych warunkach dobre jest Single Exit Point a w innych nie. Up to you.

Reklamy
Ten wpis został opublikowany w kategorii Programowanie i oznaczony tagami , , , . Dodaj zakładkę do bezpośredniego odnośnika.

8 odpowiedzi na „Single Exit Point

  1. Pingback: dotnetomaniak.pl

  2. LNG pisze:

    Czyli brak wniosków?

  3. Wnioskiem może być to, że tutaj się sprawdza to a tam tamto i nie warto na ślepo stosować (lub nie stosować) reguły Signle Exit Point.
    Dla kontrastu – gdybym pisał o Dependency Injection to bym napisał, że stosować zawsze.

  4. msz pisze:

    Właściwie to ten post nic nie wnosi 😦

  5. No cóż. Z pewnościa wpisy, które sa ostre (w sensie opinii) lepiej wychodza. Pomyślę nad jakimś RANTem 😉

  6. Ja jestem zdania, że to należy dopasować do sytuacji, ale generalnie bardziej skłaniam się ku opcji „uciekania” jak najszybciej jak nie można już kontynuować funkcji, bo jakieś dane nie pasują.

    Mamy w firmie tony starego kodu, który ma zupełnie przeciwne podejście i niestety jest to cholernie nieczytelne. Zagnieżdżenia wynikające z zastosowania tam Single Exit Point dochodzą do absurdalnych poziomów, bo każde kolejne zawołanie podrzędnej funkcji, które ma kontrole błędów zwiększa je o jeden:

    errors = Function1(parameters);
    if (errors != null)
    {
        //...
        errors = Function2(parameters);
        if (errors != null)
        {
            // ...
            errors = Function3(parameters);
            if (errors != null)
            {
                // ...
            }
            else
            {
                // error handling logic
            }
        }
        else
        {
            // error handling logic
        }
    }
    else
    {
        // error handling logic
    }
    

    Jest kilka przypadków, że właściwy kod zaczyna się prawie w połowie szerokości ekranu 😀
    Dodatkowo logika obsługi błędów rozrzuca się bardzo po kodzie i jeśli if ma stosunkowo dużo instrukcji to else z obsługą błędu jest gdzieś daleko i trzeba go szukać po klamrach.
    Oczywiście tutaj jest jeszcze inny problem, zbyt duże metody. Sam stosuje podejście, żeby jedna metoda mieściła się na jednym ekranie, żeby dało się ją w całości ogarnąć a jak się robi za duża to refactor. Wszystkich od razu nie nauczę, ale próbować będę 🙂
    Jednak mimo stosowania małych metod wole metodę ucieczki. Przepływ aplikacji jest dla mnie wtedy bardziej czytelny i też mniej miejsca zajmuje (nie zawiera dodatkowych konstrukcji else z klamrami. „Uciekam” jednak w zasadzie tylko przy obsłudze błędów, dla właściwej logiki, która musi zwrócić kilka różnych rzeczy różnymi ścieżkami już raczej stosuje SEP, choć i pewnie tutaj znajdą się wyjątki.

  7. marek pisze:

    Wg mnie SEP jest dobrym rozwiazaniem i powinno sie go stosowac – jedynym wyjatkiem jest sprawdzanie argumentow (null, puste listy itp), ale takie przypadki nalezy traktowac wlasnie jako wyjatek a nie argument, ze dwa podejscia sa dobre 🙂

  8. W większych projektach dla sprawdzanie argumentow (jeśli już sprawdzać) staram się wykorzystywać Code Contracts, a w prostych case’ach tak jak piszesz.

Możliwość komentowania jest wyłączona.