Hey, taky jsi zešílel z toho, jak vidíš tuny zanořených ifů, elsů a kdo ví čeho ještě? Pojď se toho zbavit.
TL;DR? koukni na video:
Dost často vidím kódy, podobné tomuhle zvěrstvu
Všechny příklady jsou v PHP, ale obecně to platí pro všechny ostatní jazyky, které podporují líné vyhodnocování (a často i pro ty, které ne, jen se často budou ify jinak zapisovat)
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
$retVal = '';
if ($dateDiff->y === 0) {
if ($dateDiff->days === 0) {
if ($dateDiff->h === 0) {
if ($dateDiff->i === 0) {
$retVal .= 'Right now';
} elseif ($dateDiff->i === 1) {
$retVal .= 'Minute ago';
} else {
$retVal .= 'Before ' . $dateDiff->i . ' minutes';
}
} elseif ($dateDiff->h === 1) {
$retVal .= 'An hour ago';
} else {
$retVal .= 'Before ' . $dateDiff->h . ' hours';
}
} elseif ($dateDiff->days === 1) {
$retVal .= 'Yesterday';
} else {
$retVal .= 'Before ' . $dateDiff->d . ' days';
}
} elseif ($dateDiff->y === 1) {
$retVal .= 'A year ago';
} else {
$retVal .= 'Before ' . $dateDiff->y . ' years';
}
return $retVal;
}
Ano, bohužel toto je opravdu reálný kód nasazený v produkci. Představte si, že by jste podobný kód měli upravovat? Přidat další podmínky, narvat do něj nedejbůh cykly atp.
Kódy jako je tento dodržují tzv. Single return policy, kdy se má v rámci funkce vyskytovat pouze 1 return statement a to právě na konci funkce.
Učí to tak na školách a používá se to na starých projektech v low-level programovacích jazycích, což hodně vývojářům přijde jako dobrá praktika, čemuž následně věří. Bohužel jak vidíte, tak to vede k šíleným zanořením kódu, porušení DRY principu apod.
Koukni se například na následující kód, který taktéž využívá Single return policy
public function isInMatrix(array $matrix, mixed $value): bool
{
$shouldExit = false;
foreach ($matrix as $col) {
foreach ($col as $row) {
if ($row === $value) {
$shouldExit = true;
break;
}
}
if ($shouldExit) {
break;
}
}
return $shouldExit;
}
Oproti
public function isInMatrix(array $matrix, mixed $value): bool
{
foreach ($matrix as $col) {
foreach ($col as $row) {
if ($row === $value) {
return true;
}
}
}
return false;
}
Jak vidíš, tak v prvním případě jsme dodrželi Single return policy a v druhém ji porušili, což zapříčinilo zlepšení čitelnosti kódu a ušetřili jsme 2 ify a proměnnou. Jednoznačně WIN.
Tato technika se jmenuje early return, kdy ve funkci vracíme dříve, než na jejím konci. Ovšem tato technika je doporučená používat pouze v případě, že se taktéž dodržuje single responsibility principle, který se neaplikuje jen na třídy, ale i na funkce, a funkce musí být tak krátké jak to jen jde, jelikož ve složitějších funkcích se v tom, kde je return můžeme snadno ztratit.
Dobře, funkce chci čitelné, rozšířitelné a čisté. Jak na to?
První, co s tím kódem musíme udělat je, že využijeme guard clauses, která využívá právě early return k tomu, aby se hned na začátku funkce eliminovaly všechny dopředu známé stavy, které nejsou pořebné pro další vykonávání funkce, což ji zpřehlední a taktéž často zrychlí.
Vyextrahujeme tedy takový první stav tak, že nejvyšší if otočíme a else statementy vložíme do něj a uvnitř původních else statementů můžeme dát rovnou return:
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
$retVal = '';
if ($dateDiff->y > 0) {
if ($dateDiff->y === 1) {
return 'A year ago';
} else {
return 'Before ' . $dateDiff->y . ' years';
}
}
if ($dateDiff->days === 0) {
if ($dateDiff->h === 0) {
if ($dateDiff->i === 0) {
$retVal .= 'Right now';
} elseif ($dateDiff->i === 1) {
$retVal .= 'Minute ago';
} else {
$retVal .= 'Before ' . $dateDiff->i . ' minutes';
}
} elseif ($dateDiff->h === 1) {
$retVal .= 'An hour ago';
} else {
$retVal .= 'Before ' . $dateDiff->h . ' hours';
}
} elseif ($dateDiff->days === 1) {
$retVal .= 'Yesterday';
} else {
$retVal .= 'Before ' . $dateDiff->d . ' days';
}
return $retVal;
}
To stejné provedeme s druhým vnořeným ifem starajícím se o dny:
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
$retVal = '';
if ($dateDiff->y > 0) {
if ($dateDiff->y === 1) {
return 'A year ago';
} else {
return 'Before ' . $dateDiff->y . ' years';
}
}
if ($dateDiff->days > 0) {
if ($dateDiff->days === 1) {
return 'Yesterday';
} else {
return 'Before ' . $dateDiff->d . ' days';
}
}
if ($dateDiff->h === 0) {
if ($dateDiff->i === 0) {
$retVal .= 'Right now';
} elseif ($dateDiff->i === 1) {
$retVal .= 'Minute ago';
} else {
$retVal .= 'Before ' . $dateDiff->i . ' minutes';
}
} elseif ($dateDiff->h === 1) {
$retVal .= 'An hour ago';
} else {
$retVal .= 'Before ' . $dateDiff->h . ' hours';
}
return $retVal;
}
A nakonec i s hodinami:
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
if ($dateDiff->y > 0) {
if ($dateDiff->y === 1) {
return 'A year ago';
} else {
return 'Before ' . $dateDiff->y . ' years';
}
}
if ($dateDiff->days > 0) {
if ($dateDiff->days === 1) {
return 'Yesterday';
} else {
return 'Before ' . $dateDiff->d . ' days';
}
}
if ($dateDiff->h > 0) {
if ($dateDiff->h === 1) {
return 'An hour ago';
} else {
return 'Before ' . $dateDiff->h . ' hours';
}
}
if ($dateDiff->i === 0) {
return 'Right now';
} elseif ($dateDiff->i === 1) {
return 'Minute ago';
} else {
return 'Before ' . $dateDiff->i . ' minutes';
}
}
Teď si všimni, že poslední if vypadá trochu jinak než ostatní, ale můžeme jej ekvivalentně prepsat, aby vypadal stejně a to z:
if ($dateDiff->i === 0) {
return 'Right now';
} elseif ($dateDiff->i === 1) {
return 'Minute ago';
} else {
return 'Before ' . $dateDiff->i . ' minutes';
}
na
if ($dateDiff->i > 0) {
if ($dateDiff->i === 1) {
return 'Minute ago';
} else {
return 'Before ' . $dateDiff->i . ' minutes';
}
}
// Tady jsme v případě, že proměnná $dateDiff->i je rovna nule, jelikož kdyby nebyla, tak funkce skončí v ifu..
return 'Right now';
A výsledek:
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
if ($dateDiff->y > 0) {
if ($dateDiff->y === 1) {
return 'A year ago';
} else {
return 'Before ' . $dateDiff->y . ' years';
}
}
if ($dateDiff->days > 0) {
if ($dateDiff->days === 1) {
return 'Yesterday';
} else {
return 'Before ' . $dateDiff->d . ' days';
}
}
if ($dateDiff->h > 0) {
if ($dateDiff->h === 1) {
return 'An hour ago';
} else {
return 'Before ' . $dateDiff->h . ' hours';
}
}
if ($dateDiff->i > 0) {
if ($dateDiff->i === 1) {
return 'Minute ago';
} else {
return 'Before ' . $dateDiff->i . ' minutes';
}
}
return 'Right now';
}
Ty Toncku... Ty seš trouba. však přehlednější to možná je, ale je to na víc řádků a pořád tam máš else statementy...
Hej mlč 😀 Všechno se dozvíš 😀.
Na čase je zjednodušit tedy ty ify. Jako první můžeme úplně eliminovat vnořený else statement, jelikož pokud nadřazený if bude splněný, tak se v něm vrátí a else není potřeba.
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
if ($dateDiff->y > 0) {
if ($dateDiff->y === 1) {
return 'A year ago';
}
return 'Before ' . $dateDiff->y . ' years';
}
if ($dateDiff->days > 0) {
if ($dateDiff->days === 1) {
return 'Yesterday';
}
return 'Before ' . $dateDiff->d . ' days';
}
if ($dateDiff->h > 0) {
if ($dateDiff->h === 1) {
return 'An hour ago';
}
return 'Before ' . $dateDiff->h . ' hours';
}
if ($dateDiff->i > 0) {
if ($dateDiff->i === 1) {
return 'Minute ago';
}
return 'Before ' . $dateDiff->i . ' minutes';
}
return 'Right now';
}
Teď jsme se trochu zasekli, protože v aktuální funkci to dál zjednodušit nepůjde. Ale vidíme hodně se opakující kód, kde se mění jen hlášky a hodnota v podmínce. To znamená, že tato funkce dělá příliš mnoho věcí a měli bychom ji nějak rozdělit. Tedy opakující se kód můžeme vyextrahovat do zvláštní funkce, abychom dodrželi DRY a Single responsibility principy:
private function getVariant(int $value, string $valueOne, string $valueMany): ?string
{
if ($value > 0) {
if ($value === 1) {
return $valueOne;
}
return sprintf($valueMany, $value);
}
return null;
}
a v originální funkci ji budeme jen volat:
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
// Jedno rovná se, protože jde o přiřazení a vyhodnocení toho, co je v $value a ne o porovnání
if ($value = $this->getVariant(value: $dateDiff->y, valueOne: 'A year ago', valueMany: 'Before %d years')) {
return $value;
}
if ($value = $this->getVariant(value: $dateDiff->days, valueOne: 'Yesterday', valueMany: 'Before %d days')) {
return $value;
}
if ($value = $this->getVariant(value: $dateDiff->h, valueOne: 'An hour ago', valueMany: 'Before %d hours')) {
return $value;
}
if ($value = $this->getVariant(value: $dateDiff->i, valueOne: 'Minute ago', valueMany: 'Before %d minutes')) {
return $value;
}
return 'Right now';
}
Jak vidíme, tak původní funkce už je dost dobře čitelná, ale ještě si ji vylepšíme.
Ok, ale pořád se mi nelíbí, jak ty funkce vypadají. Nejde to ještě zjednodušit?
Jo jde. Teď se přesuneme do naší nové funkce a přepíšeme ji, aby byla ještě lepší. Vnější IF zase můžeme znegovat a vyextrahovat nahoru, pomocí guard clauses
private function getVariant(int $value, string $valueOne, string $valueMany): ?string
{
if ($value === 0) {
return null;
}
if ($value === 1) {
return $valueOne;
}
return sprintf($valueMany, $value);
}
A nakonec, pokud jsme tento typ lidí, tak v takto jednoduchých funkcích můžeme vynechat u IFů závorky a výsledek je velmi pěkný a přehledný. Já ovšem osobně závorky nechávám, aby byl kód konzistentní.
private function getVariant(int $value, string $valueOne, string $valueMany): ?string
{
if ($value === 0) return null;
if ($value === 1) return $valueOne;
return sprintf($valueMany, $value);
}
Jak vidíš, tak v této funkci jsou jen 2 ify a žádný else, což ji udělalo extra čitelnou.
Teď skočme na tu dlouhou funkci. V PHP existuje pro tyto ify zkratka, které se říká null coalescing operator
Ne každý jazyk ho má, ale jde o to, že v tomto stavu tuto funkci můžeme klidně nechat. Pokud ale máme tuto možnost, tak
tuto funkci můžeme ještě zpřehlednit:
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
$value = $this->getVariant(value: $dateDiff->y, valueOne: 'A year ago', valueMany: 'Before %d years');
$value = $value ?? $this->getVariant(value: $dateDiff->days, valueOne: 'Yesterday', valueMany: 'Before %d days');
$value = $value ?? $this->getVariant(value: $dateDiff->h, valueOne: 'An hour ago', valueMany: 'Before %d hours');
$value = $value ?? $this->getVariant(value: $dateDiff->i, valueOne: 'Minute ago', valueMany: 'Before %d minutes');
return $value ?? 'Right now';
}
Nice. Teď už to má dohromady méně řádků, než původní kód a je to taky přehlednější. Stále tu ale je nějaký problém a to ten,
že ternár je vlastně taky if, takže náš kód je sice přehlednější, ale je náročnější na vykonání, jelikož vždy provede 4 porovnání.
To se dá vyřešit tak, že úplně vynecháme proměnnou $value
, správně zazávorkujeme ternáry a tím pádem pokud první ternár projde,
tak se ostatní nevykonají, jelikož PHP umí lazy evaluation, tak se nemusíme bát, že by se vykonaly podmínky všechny.
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
return
$this->getVariant(value: $dateDiff->y, valueOne: 'A year ago', valueMany: 'Before %d years') ??
($this->getVariant(value: $dateDiff->days, valueOne: 'Yesterday', valueMany: 'Before %d days') ??
($this->getVariant(value: $dateDiff->h, valueOne: 'An hour ago', valueMany: 'Before %d hours') ??
($this->getVariant(value: $dateDiff->i, valueOne: 'Minute ago', valueMany: 'Before %d minutes') ??
'Right now')));
}
A je to. Celý kód vypadá takto:
private function getVariant(int $value, string $valueOne, string $valueMany): ?string
{
if ($value === 0) return null;
if ($value === 1) return $valueOne;
return sprintf($valueMany, $value);
}
public function getWriteDate(DateTime $created): string
{
$dateDiff = $created->diff(DateTime::from('now'));
return
$this->getVariant(value: $dateDiff->y, valueOne: 'A year ago', valueMany: 'Before %d years') ??
($this->getVariant(value: $dateDiff->days, valueOne: 'Yesterday', valueMany: 'Before %d days') ??
($this->getVariant(value: $dateDiff->h, valueOne: 'An hour ago', valueMany: 'Before %d hours') ??
($this->getVariant(value: $dateDiff->i, valueOne: 'Minute ago', valueMany: 'Before %d minutes') ??
'Right now')));
}
Výsledky:
Počet řádků | Počet ifů včetně elsů | Přehlednsot | Bezpečnost | |
---|---|---|---|---|
Původní kód | 33 | 12 | Není | Nebezpečné |
Nový kód | 18 | 6 | Luxusní | Bezpečné |
Vidíme, že výsledný kód je o polovinu kratší a přehlednější, což velmi přispívá k bezpečnosti díky principům less code = more security a more complex = less secure které definují nepřímou úměru mezi složitostí kódu a bezpečností a množstvím čehokoliv a bezpečností.
Jediná nevýhoda nového kódu je, že se do funkce předávají 2 řetězce a to zbytečně, což zpomaluje kód, jelikož tyto řetězce se kopírují. Tahle optimalizace ale již není součástí tohoto tipu, tedy přijít na to, jak to vylepšit, aby se nemusely předávat řetězce v argumentech je již na tobě. A neboj. Je to jednodušší, než se zdá.
Tak se snaž a já jdu barvit trávu na zeleno.