return result; //anti-pattern

Koden du förhoppningsvis aldrig behöver se:

Result foo() {
   Result result = SOME_CONSTANT_FOR_EXAMPLE_NULL;
   code...
   code...
   code...
   return result; //anti-pattern
}

Varför är det så illa?

Exempel 1:

Result foo() {
   Result result = SOME_CONSTANT_FOR_EXAMPLE_NULL;
   code...
   code...
   if (condition1) {
     result = value1;
   }
   code...
   code...
   if (condition2) {
     result = value2;
   }
   code...
   code...
   return result; //anti-pattern
}

Ser ni problemet? Om båda if-satserna tas, så är det oklart om funktionen returnerar vad man förväntar sig. I skarpa fall, med många olika utvecklare och flera lager av ”ful-nästling”, blir det nästan omöjligt att tolka koden, man får gissa vad den gör.

Exempel 2:

Result foo() {
   Result result = SOME_CONSTANT_FOR_EXAMPLE_NULL;
   try {
     code...
     code...
     result = value1;
     code...
     code...
   } catch (SomeException e) {
     code...
     code...
   }
   code...
   code...
   return result; //anti-pattern
}

Vad returnerar metoden ovan egentligen? Är resultatet pålitligt för fem öre? Ännu värre fall: funktioner där result tilldelas värden mer än en gång, t.ex. både håller temporärt resultat och det som skall returneras.

Eller klassikern, exempel 3:

Result foo() {
   Result result = SOME_CONSTANT_FOR_EXAMPLE_NULL;
   code...
   code...
   if (contion1) {
     code...
     code...
     if (contion2) {
       code...
       code...
     }
     else {
       code...
       code...
     }
     code...
     code...
   }
   else {
     code...
     code...
   }
   code...
   code...
   return result; //anti-pattern
}

Ser ni vad metoden ovan returnerar? Doh! Konstant return! Och denna är mycket vanligare än man kan tro. Speciellt i funktioner som returnerar boolean och normaltfallet är det som alltid returneras.

Tips från coachen: Ta bort variabeln result! Om du någon gång funderat över hur man gör return så är svaret: kör return! I vissa varianter av C kan problemet vara komplext och kräva den här typen av kod, men måste man ha den så skall man tänka sig för. I bland annat Java och C# finns en korrekta sätt att hantera de svårare fallen, med t.ex. using eller try-finally:

Result foo() {
   try {
     code...
     code...
     return value1;
   } catch (SomeException e) {
     code...
     code...
     return SOME_CONSTANT_FOR_EXAMPLE_NULL;
   }
   finally {
     // bra ställe att lägga upprensning som krävs i samtliga fall
     code...
     code...
   }
   // är all annan kod rätt behövs inte en return här
}

eller:

Result foo() {
   using (någon resurs) {
     code...
     code...
     return value1;
   }
}

eller:

Result foo() {
  if (condition1) {
    return value1;
  }
  else if (condition2) {
    if (condition3) {
      return value3;
    }
    return value2;
  }
  else {
    return value4;
  }
}

Osv. Det blir ett tydligt flöde i funktionerna, man kan se vad som returneras och slipper oroa sig för märkliga sidoeffekter.

Google:ar man så visar det sig att det faktiskt finns många utvecklare som gillar att använda single function exit point anti-pattern, med bland annat följande argument:

  • Man gjorde så tidigare. *doh?*. På den tiden ANSI-C och QBasic var de språken man använde, kanske.
  • Det är lättare att läsa om funktionen är lång. *doh!* 1. Det är inte sant. Refaktorerar man denna typ av kod så får man nästan alltid mångfaldigt mer lättläst kod. 2. Är funktionen lång, så har man förmodligen andra problem. Refaktorera koden så att funktionerna blir hanterbart långa.

Man kan ana sig till att många utvecklare inte insett storheten i att ha sluta använda ett pattern som slår ihjäl kompilatorn / utvecklingsmiljöns förmåga att hitta missade kodvägar; varför koda på ett sätt som förstör för verktygens möjligheter att hitta buggar i din kod? Med multiple exit points blir det mycket svårare att skriva kod som inte täcker upp alla specialfall.

Att man dessutom tror att långa och svårbegripliga funktioner, där en variabel return på något sätt räddar koden, skulle vara åtråvärt… kan förmodligen bara förklaras med att utvecklarna aldrig praktiserat refaktorering. Med en modern utvecklingsplatform identifierar man kod som hör ihop, väljer att bryta ut metoden. Efter en stund är 200-raders oläslig funktion med single point return struktur nedbruten till kanske 5 – 15 små begripliga funktioner… och ingen av dem bygger på single point return anti-pattern för att ”rädda dem”.

Annonser

Kommentera

Fyll i dina uppgifter nedan eller klicka på en ikon för att logga in:

WordPress.com Logo

Du kommenterar med ditt WordPress.com-konto. Logga ut / Ändra )

Twitter-bild

Du kommenterar med ditt Twitter-konto. Logga ut / Ändra )

Facebook-foto

Du kommenterar med ditt Facebook-konto. Logga ut / Ändra )

Google+ photo

Du kommenterar med ditt Google+-konto. Logga ut / Ändra )

Ansluter till %s