Czy to już paranoja?

a zaczęło się to tak...

Wydajność mojego zespołu zwiększyła się w ostatnich miesiącach kilkukrotnie. Oczywiście jest to powód do zadowolenia, ale generuje to również wiele code review, które wcześniej czy później trzeba przejrzeć. Co prawda, nie wszyscy muszą oglądać wszystko, ale tak czy inaczej, ich ilość ostatnimi czasy jest przytłaczająca. Zazwyczaj rano przy kawie przeglądam ich listę i po kolei, jeden po drugim, zamykam bądź oglądam, w zależności od tego, ile osób przede mną już miało przyjemność patrzyć na ten kod. Tak też zrobiłem ostatnio. Gorąca, czarna, parzona i aromatyczna kawa powoli cuciła mnie i przywracał do świata żywych, a ja w między czasie przedzierałem się przez kolejne linijki kodu.

W pewnym momencie trafiłem na bardziej złożone zadanie i po ilości klas byłem w stanie stwierdzić, że zrozumienie wszystkiego będzie wymagało ode mnie trzeźwości umysłu i skupienia, o jakie ciężko we wczesnych godzinach porannych. Ale któż powiedział, że życie lekkie jest:) Łyk, już tylko ciepłego, napoju i do dzieła.

przedzierając się przez kod

Muszę przyznać, że wszystko było naprawdę świetnie zrobione i tylko jeden mały szczegół przykuł moją uwagę i nie dawał mi spokoju. A że upierdliwa ze mnie osoba, to pozwoliłem sobie na zaznaczenie w odpowiednim miejscu defecta i wyartykułowanie moich uwag.

Poniżej przykład kodu. Usunąłem wszystkie nieistotne dla wpisu rzeczy oraz pozmieniał nazwy wedle uznania, ale sama struktura pozostała:

<?php
abstract class MyAbstract
{
  protected $_error;

  public function execute()
  {
     $this->_myTemplateMethod();
     // code
  }

  // class body

  abstract protected function _myTemplateMethod();
}

class Class1 extends MyAbstract
{
  private $_id;

  protected function _myTemplateMethod()
  {
     if (!($this->_error xor $this->_id))
        throw new Exception('Wrong values given');
  }

  // class body
}

class Class2 extends MyAbstract
{
  private $_status;

  protected function _myTemplateMethod()
  {
     if (!($this->_error xor $this->_status))
        throw new Exception('Wrong values given');
  }

  // class body
}

Są wzorce, są abstrakcje, wszystko świetnie, ale...

ten upierdliwy człek

Chyba już dałem się poznać na tyle w tych wszystkich wpisach, które Wam zaserwowałem, że jesteście w stanie szybko stwierdzić, co tak wyraźnie rzuciło mi się w oczy:) Chodziło o implementację metody szablonowej, która w dwóch klasach wyglądała prawie tak samo. Dlatego też pokusiłem się o zasugerowanie zmiany w kodzie:

<?php
abstract class MyAbstract
{
  private $_error;

  public function execute()
  {
     $this->_doSomething();
     // code
  }

  abstract protected function _getRequiredValue();

  private function _doSomething()
  {
     if (!($this->_error xor $this->_getRequiredValue()))
        throw new Exception('Wrong values given');
  }

  // class body
}

class Class1 extends MyAbstract
{
  private $_id;

  protected function _getRequiredValue()
  {
     return $this->_id;
  }

  // class body
}

class Class2 extends MyAbstract
{
  private $_status;

  protected function _getRequiredValue()
  {
     return $this->_status;
  }

  // class body
}

Co prawda okazało się, że trzeba było ten kod jeszcze odrobinę zmienić z powodu następnego zadania, nad którym właśnie mój kolega pracował, ale idea pozostała.

Czym uzasadniam powyższe kroki?

• atrybut klasy abstrakcyjnej zmienił widoczność na private, a więc klasy pochodne nie mają potrzeby znajomości atrybutów rodzica
• brak duplikacji w kodzie, ponieważ wszystko, co się powtarzało zostało wyniesione "do góry"
• wcześniejszy punkt pociąga za sobą jeszcze jedną istotną rzecz - w przypadku błędu, mamy tylko jedno miejsce, gdzie należy kod zmienić

przerost formy nad treścią?

Czy rzeczywiście?
Jak odpisywałem ostatnio pod jednym z wpisów (luźny cytat:P): "to, że nie żyjemy w idealnym świecie i nie ma idealnych środowisk, nie oznacza, że do tego ideału nie można dążyć". Oznacza to tyle, że jeżeli coś da się zrobić lepiej i wiemy o tym, to właśnie tak należy to wykonać.

"Rozdrabniamy się i tracimy czas na głupoty", ale czy na pewno? W tym momencie były tylko dwie klasy, ale trzecia była już w drodze, a co w przypadku, gdyby powstało ich dwadzieścia i w metodzie szablonowej (tej pierwotnej) byłby błąd w implementacji? Dwadzieścia klas do poprawy i do tego jeszcze testy. Dzięki usuwaniu duplikacji unikamy takiego problemu.

Ktoś mi może zarzucić, że zawsze powtarzam, aby nie gdybać o przyszłości i nie zastanawiać się "co by było gdyby" i "co może się stać". Z tym, że w tym przypadku to nie jest gdybanie. To jest zarządzanie ryzykiem. Jeżeli widzimy miejsce, które może być potencjalnym źródłem kłopotów, to nawet jeżeli uważamy, że "to tylko detal", to warto poświęcić te kilka chwil na usunięciu tego szczegółu, bo takie banały są podstawą wszystkich poważniejszych problemów.

czy to już paranoja?

Czy naprawdę wszędzie już dostrzegam niedoskonałości kodu i nawet obok czegoś dobrego nie potrafię przejść bez słowa krytyki?
Niestety nie, czasami nie udaje mi się czegoś wychwycić, czasami sam popełniam błędy, po których wytknięciu najchętniej zapadłbym się pod ziemię.
Co jednak nie zmienia faktu, że sukcesywnie zmierzam do takiego stanu:)

Czy to jednak źle? Czy to już rzeczywiście paranoja?