Mirin webspace

Nejbohatší život má ten, kdo žije s minimem nároků

9. 8. 2014 - Komentáře (4) PHP

Firemní PHP žigulík

Jednou jsem měl v práci přidat to stávajícho systému novou vlastnost, o co přesně jde je celkem jedno, ale součástí stávajícího řešení je metoda, kterou jsem označil za prásarnu, nejdřív se podíváme, jak ten kód vypadá v IDE, protože to je už dnes ve firmách, jež na nějakém rozsáhlejším PHP kódu zavisí, téměř nutnost.

 parameters WTF

Bez hlubšího zkoumání si člověk řekne WTF, k čemu tam je ta hromada argumentů, když se vůbec nepoužívají (šedivé argumenty IDE označuje jako nepoužité). Ale pozor, raději si to přepíšeme "na papír" a hned je jasnější, o co kráčí.

public function addItem($sku, $name, $price, $category = null, $quantity = 1) {
 $product = array();
 foreach (array('sku', 'name', 'category', 'price', 'quantity') as $arg) {
  if ($$arg !== null) {
   $product[$arg] = $$arg;
  }
 }
 
 if (empty($this->transaction['transactionProducts'])) {
  $this->transaction['transactionProducts'] = array();
 }
 $this->transaction['transactionProducts'][] = $product;
 return $this;
}

Celé je to v podstatě o úplně triviální věci, nasypat hodnoty z argumentů do pole a to pak někam uložit. Nicméně i tahle triviální věc se tady dělá "trikováním", jména proměnných, v našem případě dokonce argumentů, jejichž obsah potřebujeme uložit, jsou v poli řetězců, a výsledná hodnota se ukládá pomocí speciální konstrukce variable variable - $$arg.

Proč uvádím "dokonce argumentů"? Protože když se změní jméno argumentu, tak je tady nepřímá závislost nějaké interní struktury funkce na jméně argumentu, není vůbec triviální odhalit co a jak opravit.

Zkušenější programátor v php by také měl vědět, že dynamické konstrukce typu variable variable, variable functions, jsou takový zakuklený eval, tím pádem pomalé, žádnou optimalizaci php dělat nemůže, musí udělat lookup všude možně, aby nalezl odpovídající proměnnou, funkci atd.

Zkrátka přišlo mi to jako dost typická ukázka práce mazaného a líného programátora. I když tady to je spíš špatný pokus být mazaný. Tupé řešení

public function addItem($sku, $name, $price, $category = null, $quantity = 1) {
  $product = array(
   "sku" => $sku,
   "name" => $name,
   "price" => $price,
   "category" => $category,
   "quantity" => $quantity);
 
  foreach ($product as $parameter => $value) {
   if ($value === null) unset($product[$parameter]);
  }
 
  if (empty($this->transaction['transactionProducts'])) {
   $this->transaction['transactionProducts'] = array();
  }
  $this->transaction['transactionProducts'][] = $product;
  return $this;
}

Se určitě dá vylepšit, ale už to rozhodně není prasečina a trikování. IDE s ním nemá problém, přejmenovat parametr je triviální a bude to určitě i celé mnohem rychleji fungovat.

Pravda je, že stávající řešení je funkční, kdyby to bylo v nějaké jednorázovém skriptu, pomocné utilitě, testu atp., neřeknu, ale tohle je produkční kód, který se za den vykoná velmi často, tak jsem se ozval, že mi to připadá jako prasácké trikování. Autor kódu je současný manažer, který si občas zaprogramuje v php, dříve byl i pokročilý java programátor. Ten odpověděl, že to jen zkopíroval (a vskutku jsem to pak našel ještě na několika dalších místech), ale nepřijde mu, že to není OK, a vůbec že nemá čas to řešit. No a náš vrchní vývojář - "samozřejmě, že to je OK a není nejmenší důvod to předělávat. Máme dost jiné práce".

Pokud u vás píšete testy a neklikáte pořád dokola v prohlížeči, píše se rozumná dokumentace, když se něco nahackuje (naprasí), tak se neplácáte po zádech, jak jste to zmákli, ale pravidelně to čistíte a nenecháváte na "někdy" a nedejbože i funguje continuos integration a dokonce děláte i smysluplné code review, které zlepšuje kvalitu práce vás i kolegů, tak to si můžete pískat jako Don Číčo a nemusíte žrať žiadnu rižu, protože tu bižu, kterou v práci potřebujete, už dávno máte.


Komentáře (4)

  1. pp - 11. 8. 2014 21:55

    Panejo, to máte nějaké osobní předsevzetí psát první a poslední odstavec jednou nečitelnou větou? Z pohledu čitelnosti je to stejná prasárna, jako onen php kód: abych vůbec pochopil smysl, musím si to "přepsat na papír" a rozdělit na smysluplná souvětí a krátké věty.

  2. koubel - 13. 8. 2014 15:08

    Díky za text review, to mi už pár lidí také řeklo, tak snad někdy příště. I když tou frekvencí jeden článek za rok to asi moc nevypiluju

  3. bjoe - 11. 7. 2016 14:53

    Trochu blbá otázka. Kde se naučit to vaše super programování. Pořád na to nic nemůžu najít. Díky za případnou reakci.

  4. koubel - 26. 7. 2016 10:40

    [3] teď tedy nevím, jestli jste to myslel posměšně nebo vážně. Žádné super programování to rozhodně není, stačí si najít dobrý kolektiv, kde se alespoň trochu dbá o to, aby vývojáři neustrnuli a zbytek už přijde sám.

Komentáře jsou uzavřeny.