Najlepsza praktyka dotycząca if / return

Chcę wiedzieć, co jest uważane za lepszy sposób zwrotu, gdy mam ifoświadczenie.

Przykład 1:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }
   else
   {
      myString = "Name " + myString;
      // Do something more here...
      return true;
   }
}

Przykład 2:

public bool MyFunction()
{
   // Get some string for this example
   string myString = GetString();

   if (myString == null)
   {
      return false;
   }

   myString = "Name " + myString;
   // Do something more here...
   return true;
}

Jak widać w obu przykładach, funkcja zwróci, true/falseale czy warto umieścić elseinstrukcję jak w pierwszym przykładzie, czy lepiej jej nie umieszczać?

Przykład 2 jest znany jako blok ochronny. Lepiej jest odpowiednio wcześnie zwrócić / wyrzucić wyjątek, jeśli coś poszło nie tak (zły parametr lub nieprawidłowy stan). W normalnym przepływie logicznym lepiej jest użyć przykładu 1

Moim osobistym stylem jest używanie pojedynczego ifdla bloków ochronnych i if/ elsew rzeczywistym kodzie przetwarzania metody.

W tym przypadku używasz myString == nullwarunku ochronnego, więc chciałbym użyć jednego ifwzorca.

Rozważ kod, który jest nieco bardziej skomplikowany:

Przykład 1:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }
    else{
        //processing block
        myString = escapedString(myString);

        if (myString == "foo"){
            //some processing here
            return false;
        }
        else{
            myString = "Name " + myString;
            //other stuff
            return true;
        }
    }
}

Przykład 2:

public bool MyFunction(myString: string){

    //guard block
    if (myString == null){
        return false;
    }

    //processing block
    myString = escapedString(myString);

    if (myString == "foo"){
        //some processing here
        return false;
    }
    else{
        myString = "Name " + myString;
        //other stuff
        return true;
    }
}

W przykładzie 1 zarówno strażnik, jak i reszta metody są w formie if/ else. Porównaj to z przykładem 2, w którym blok ochronny jest w pojedynczej ifformie, podczas gdy reszta metody używa formy if/ else. Osobiście uważam przykład 2 za łatwiejszy do zrozumienia, podczas gdy przykład 1 wygląda na niechlujny i nadmiernie wcięty.

Zauważ, że jest to wymyślony przykład i że możesz użyć else ifinstrukcji, aby go wyczyścić, ale chcę pokazać różnicę między blokami ochronnymi a rzeczywistym kodem przetwarzania funkcji.

Przyzwoity kompilator i tak powinien wygenerować te same dane wyjściowe dla obu. Jedynym powodem użycia jednego lub drugiego jest osobiste preferencje lub dostosowanie się do stylu istniejącego kodu.

osobiście wolę drugą metodę. Wydaje mi się, że jest krótszy, ma mniej wcięć i jest łatwiejszy do odczytania.

Moja osobista praktyka jest następująca:

• Nie lubię funkcji z kilkoma punktami wyjścia, trudno mi było utrzymać i śledzić, modyfikacje kodu czasami psują wewnętrzną logikę, ponieważ z natury jest nieco niechlujna. Gdy jest to złożone obliczenie, tworzę wartość zwracaną na początku i zwracam na końcu. Zmusza mnie to do uważnego śledzenia każdej ścieżki if-else, przełącznika itp., Ustawiania wartości poprawnie w odpowiednich lokalizacjach. Poświęcam również trochę czasu na podjęcie decyzji, czy ustawić domyślną wartość zwracaną, czy pozostawić ją niezainicjowaną na początku. Ta metoda pomaga również, gdy zmienia się logika lub typ lub znaczenie zwracanej wartości.

Na przykład:

public bool myFunction()
{
   // First parameter loading
   String myString = getString();

   // Location of "quick exits", see the second example
   // ...

   // declaration of external resources that MUST be released whatever happens
   // ...

   // the return variable (should think about giving it a default value or not) 
   // if you have no default value, declare it final! You will get compiler 
   // error when you try to set it multiple times or leave uninitialized!
   bool didSomething = false;

   try {
     if (myString != null)
     {
       myString = "Name " + myString;
       // Do something more here...

       didSomething = true;
     } else {
       // get other parameters and data
       if ( other conditions apply ) {
         // do something else
         didSomething = true;
       }
     }

     // Edit: previously forgot the most important advantage of this version
     // *** HOUSEKEEPING!!! ***

   } finally {

     // this is the common place to release all resources, reset all state variables

     // Yes, if you use try-finally, you will get here from any internal returns too.
     // As I said, it is only my taste that I like to have one, straightforward path 
     // leading here, and this works even if you don't use the try-finally version.

   }

   return didSomething;
}

• Jedyny wyjątek: „szybkie wyjście” na początku (lub w rzadkich przypadkach w trakcie procesu). Jeśli rzeczywista logika obliczeniowa nie jest w stanie poradzić sobie z pewną kombinacją parametrów wejściowych i stanów wewnętrznych lub ma łatwe rozwiązanie bez uruchamiania algorytmu, nie pomaga zamknięcie całego kodu (czasem głębokiego) w blokach. Jest to „wyjątkowy stan”, nie będący częścią podstawowej logiki, więc muszę wyjść z obliczeń, jak tylko go wykryję. W tym przypadku nie ma innej gałęzi, w normalnych warunkach wykonanie po prostu trwa. (Oczywiście „wyjątkowy stan” jest lepiej wyrażony poprzez zgłoszenie wyjątku, ale czasami jest to przesada.)

Na przykład:

public bool myFunction()
{
   String myString = getString();

   if (null == myString)
   {
     // there is nothing to do if myString is null
     return false;
   } 

   myString = "Name " + myString;
   // Do something more here...

   // not using return value variable now, because the operation is straightforward.
   // if the operation is complex, use the variable as the previous example.

   return true;
}

Reguła „jednego wyjścia” pomaga również wtedy, gdy obliczenia wymagają zewnętrznych zasobów, które musisz zwolnić, lub stwierdza, że ​​musisz zresetować przed opuszczeniem funkcji; czasami są one dodawane później podczas programowania. Przy wielu wyjściach w algorytmie znacznie trudniej jest poprawnie rozszerzyć wszystkie gałęzie. (A jeśli mogą wystąpić wyjątki, wydanie / reset należy również umieścić w bloku końcowym, aby uniknąć skutków ubocznych w rzadkich wyjątkowych przypadkach ...).

Twoja sprawa wydaje się należeć do kategorii „szybkie wyjście przed prawdziwą pracą” i napisałbym ją tak, jak w twojej wersji z Przykładu 2.

Wolę stosować bloki ochronne tam, gdzie mogę, z dwóch powodów:

1. Pozwalają na szybkie wyjście z powodu określonych warunków.
2. Usuń konieczność skomplikowanych i niepotrzebnych instrukcji w dalszej części kodu.

Ogólnie rzecz biorąc wolę widzieć metody, w których podstawowa funkcjonalność metody jest jasna i minimalna. Bloki ochronne pomagają wizualnie to zrobić.

Podoba mi się podejście „Fall Through”:

public bool MyFunction()
{
   string myString = GetString();

   if (myString != null)
   {
     myString = "Name " + myString;
     return true;
    }
    return false;
}

Akcja ma określony warunek, wszystko inne to tylko domyślny powrót „przechodzący”.

Jeśli mam jeden warunek if, nie spędziłbym zbyt wiele czasu na rozmyślaniu o tym stylu. Ale jeśli mam wiele warunków ochrony, wolałbym style2

Zrób to. Załóżmy, że testy są złożone i naprawdę nie chcesz ich wiązać w jeden warunek jeśli-ORed, aby uniknąć złożoności:

//Style1
if (this1 != Right)
{ 
    return;
}
else if(this2 != right2)
{
    return;
}
else if(this3 != right2)
{
    return;
}
else
{
    //everything is right
    //do something
    return;
}

przeciw

//Style 2
if (this1 != Right)
{ 
   return;
}
if(this2 != right2)
{
    return;
}
if(this3 != right2)
{
    return;
}


//everything is right
//do something
return;

Oto dwie główne zalety

1. Oddzielasz kod w jednej funkcji na dwa wizualnie logiczne bloki: górny blok walidacji (warunki ochrony) i dolny blok uruchamialnego kodu.

2. Jeśli musisz dodać / usunąć jeden warunek, zmniejszasz swoje szanse na zepsucie całej drabiny if-elseif-else.

Kolejną niewielką zaletą jest to, że masz o jeden zestaw mniejszej liczby aparatów ortodontycznych.

Powinno to być kwestią, która brzmi lepiej.

If condition
  do something
else
  do somethingelse

wyraża się wtedy lepiej

if condition
  do something
do somethingelse

w przypadku małych metod nie będzie to wielka różnica, ale w przypadku większych metod złożonych może być trudniejszy do zrozumienia, ponieważ nie będzie odpowiednio oddzielny

Uważam przykład 1 za irytujący, ponieważ brakująca instrukcja return na końcu funkcji zwracającej wartość natychmiast wyzwala flagę „Czekaj, coś jest nie tak”. W takich przypadkach wybrałbym przykład 2.

Jednak zwykle jest więcej zaangażowanych, w zależności od celu funkcji, np. Obsługa błędów, rejestrowanie itp. Więc jestem z odpowiedzią Loranda Kedvesa na ten temat i zwykle mam jeden punkt wyjścia na końcu, nawet kosztem dodatkowej zmiennej flagi. W większości przypadków ułatwia konserwację i późniejsze rozszerzenia.

Kiedy twoja ifinstrukcja zawsze zwraca, nie ma powodu, aby używać innego dla pozostałego kodu w funkcji. Takie postępowanie dodaje dodatkowe linie i dodatkowe wcięcia. Dodanie niepotrzebnego kodu utrudnia czytanie i, jak wiadomo, czytanie kodu jest trudne .

W twoim przykładzie elsejest to oczywiście niepotrzebne, ALE ...

Podczas szybkiego przeglądania linii kodu oczy zwykle patrzą na nawiasy klamrowe i wgłębienia, aby uzyskać pojęcie o przepływie kodu; zanim faktycznie osiądą na samym kodzie. Dlatego pisanie elsei wstawianie nawiasów klamrowych i wcięć wokół drugiego bloku kodu sprawia, że ​​czytelnik szybciej widzi, że jest to sytuacja „A lub B”, w której jeden lub drugi blok będzie działał. Oznacza to, że czytelnik widzi nawiasy klamrowe i wgłębienia, ZANIM zobaczą returnpo inicjałach if.

Moim zdaniem jest to jeden z tych rzadkich przypadków, w których dodanie czegoś nadmiarowego do kodu sprawia, że ​​jest on WIĘCEJ czytelny, a nie mniej.

Wolałbym przykład 2, ponieważ natychmiast widać, że funkcja coś zwraca . Ale nawet więcej, wolałbym wrócić z jednego miejsca, takiego jak to:

public bool MyFunction()
{
    bool result = false;

    string myString = GetString();

    if (myString != nil) {
        myString = "Name " + myString;

        result = true;
    }

    return result;
}

W tym stylu mogę:

1. Zobacz od razu, że coś zwracam.

2. Złap wynik każdego wywołania funkcji za pomocą tylko jednego punktu przerwania.

Mój osobisty styl zwykle odchodzi

function doQuery(string) {
    if (!query(string)) {
        query("ABORT");
        return false;
    } // else
    if(!anotherquery(string) {
        query("ABORT");
        return false;
    } // else
    return true;
}

Używanie skomentowanych elseinstrukcji w celu wskazania przebiegu programu i zachowania jego czytelności, ale unikanie masywnych wcięć, które mogą łatwo dotrzeć na ekran, jeśli zaangażowanych jest wiele kroków.

Napisałbym

public bool SetUserId(int id)
{
   // Get user name from id
   string userName = GetNameById(id);

   if (userName != null)
   {
       // update local ID and userName
       _id = id;
       _userNameLabelText = "Name: " + userName;
   }

   return userName != null;
}

Wolę ten styl, ponieważ najbardziej oczywiste jest, że wrócę userName != null.

Moją ogólną zasadą jest albo wykonanie zwrotu „bez” (głównie w przypadku błędów), albo wykonanie pełnego łańcucha „jeśli-inaczej-jeśli” przy wszystkich możliwościach.

W ten sposób unikam zbytniej fantazji przy rozgałęzianiu, ponieważ za każdym razem, gdy to robię, koduję logikę aplikacji w moich ifs, co utrudnia ich utrzymanie (na przykład, jeśli wyliczenie OK lub ERR i piszę wszystkie moje ifs korzystając z faktu, że! OK <-> ERR staje się problemem w tyłku, aby dodać trzecią opcję do wyliczenia następnym razem).

W twoim przypadku użyłbym na przykład zwykłego, jeśli „return if null” z pewnością będzie wspólnym wzorcem i jest mało prawdopodobne, że będziesz musiał się martwić o trzecią możliwość (oprócz null / not null) w przyszłości.

Jeśli jednak test był czymś, co sprawdziło bardziej szczegółową kontrolę danych, popełniłbym błąd w kierunku pełnego „jeśli-inaczej”.

Myślę, że w Przykładzie 2 jest wiele pułapek, które mogą prowadzić do niezamierzonego kodu. Pierwszy punkt zainteresowania jest tutaj oparty na logice otaczającej zmienną „myString”. Dlatego, mówiąc wprost, wszystkie testy warunków powinny odbywać się w bloku kodu uwzględniającym znaną i domyślną / nieznaną logikę.

Co jeśli później kod został przypadkowo wprowadzony do przykładu 2, który znacząco zmienił wynik:

   if (myString == null)
   {
      return false;
   }

   //add some v1 update code here...
   myString = "And the winner is: ";
   //add some v2 update code here...
   //buried in v2 updates the following line was added
   myString = null;
   //add some v3 update code here...
   //Well technically this should not be hit because myString = null
   //but we already passed that logic
   myString = "Name " + myString;
   // Do something more here...
   return true;

Myślę, że elseblok bezpośrednio po sprawdzeniu wartości zerowej spowodowałby, że programiści, którzy dodali ulepszenia do przyszłych wersji, dodają całą logikę razem, ponieważ teraz mamy ciąg logiki, który był niezamierzony dla oryginalnej reguły (zwracany, jeśli wartość jest zero).

Wierzę w to mocno niektórym z wytycznych C # na Codeplex (link do tego tutaj: http://csharpguidelines.codeplex.com/ ), które zawierają następujące informacje:

„Dodaj opisowy komentarz, jeśli domyślny blok (inny) ma być pusty. Ponadto, jeśli ten blok nie zostanie osiągnięty, wyrzuć wyjątek InvalidOperationException, aby wykryć przyszłe zmiany, które mogą wystąpić w istniejących przypadkach. Zapewnia to lepszy kod, ponieważ wszystkie ścieżki, które kod może przemierzać, zostały przemyślane. ”

Myślę, że dobrą praktyką programistyczną jest stosowanie takich bloków logicznych, aby zawsze dodawać blok domyślny (jeśli-inaczej, case: default), aby jawnie uwzględniać wszystkie ścieżki kodu i nie pozostawiać kodu otwartego na niezamierzone konsekwencje logiczne.