Tydzień temu ogłosiłem, że chciałbym zacząć serię gdzie będę przeglądał Wasze repozytoria. Nastawiony byłem na to, że dostanę głównie repozytoria osób początkujących, które dopiero uczą się jak pisać poprawny kod. Jednak na sam początek serii jestem pozytywnie zaskoczony. Otóż odezwał się do mnie Daniel, który podesłał link do swojego repozytorium. Kiedy je otworzyłem od razu wiedziałem, że dostałem przykład repo, które mogę pokazać jako ilustrację wzorowo przygotowanego projektu.

Od razu mogę powiedzieć, że jeżeli ktoś z Was zastanawia się jak powinien wyglądać kod na GitHubie pasujący do tego co pisałem w poście o 4 błędach początkujących piszących swoje CV to tutaj macie odpowiedź.

Link do repozytorium Daniela: https://github.com/T-Regx/T-Regx

Krótko o kodzie

Projekt, który dostałem jest napisany w języku PHP także nie jest to język, w którym poruszam się na co dzień. Z tego powodu nie mogłem wyłapać jakichś specyficznych dla języka błędów.

Co Wam przychodzi na myśl kiedy słyszycie o kodzie pisanym w PHP? Jeżeli macie w głowie tylko spagetti-kod pomieszany z HTMLem i pisanymi z palca zapytaniami SQL wciśniętymi w losowych miejscach to otwierając projekt Daniela mocno się zawiedziecie. Jest to bardzo dobrze napisany OBIEKTOWY kod. Z resztą wystarczy spojrzeć na niego:

<?php
namespace TRegx\Replace;
use InvalidArgumentException;
class ReplaceLimit
{
    /** @var callable */
    private $patternFactory;
    public function __construct(callable $patternFactory)
    {
        $this->patternFactory = $patternFactory;
    }
    public function all(): ReplacePattern
    {
        return call_user_func($this->patternFactory, -1);
    }
    public function first(): ReplacePattern
    {
        return call_user_func($this->patternFactory, 1);
    }
    public function only(int $limit): ReplacePattern
    {
        if ($limit < 0) {
            throw new InvalidArgumentException("Negative limit $limit");
        }
        return call_user_func($this->patternFactory, $limit);
    }
}

Gdyby nie znacznik <?php na początku pliku to można by nie poznać, że ten kod powstał w języku, który z różnych powodów jest uważany za bałaganiarski.

Przeglądając praktycznie całe repozytorium nie znalazłem metody, która byłaby dłuższa niż kilkanaście linijek. Duży plus za to bo mimo, że funkcje są krótkie to kod nie jest przesadnie rozdmuchany. A z własnego doświadczenia wiem, że nie jest to proste. Sam łapię się czasami na tym, że chcąc zachować kompaktowe funkcje dodaję mnóstwo warstw w kodzie tylko po to, żeby gdzieś upchnąć całą logikę. Dlaczego warto pisać krótkie funkcje możecie przeczytać np. TUTAJ. No i oczywiście w znanej książce „Czysty kod” Wujka Boba.

Praktycznie wszystkie nazwy funkcji i zmiennych są na tyle dobrze nazwane, że nie miałem problemu z odnalezieniem się w kodzie. A jak wspomniałem PHP nie jest językiem, w którym siedzę. Mimo to od razu wiedziałem co z czym jak działa.

Warto również zwrócić uwagę na enkapsulację obiektów. Wszędzie wszystkie potrzebne pola w klasach są ustawiane za pomocą konstruktora. Żadnych setterów, służących do składania wartości w całość, żadnych getterów do wszystkich możliwych pól tylko po to żeby były. Wszystko jest sterowane przez ograniczony do minimum zestaw metod:

<?php
namespace TRegx\Match;
class ReplaceMatch extends Match
{
    /** @var int */
    private $offsetModification;
    /** @var int */
    private $limit;
    public function __construct(string $subject, int $index, array $matches, int $offsetModification, int $limit)
    {
        parent::__construct($subject, $index, $matches);
        $this->offsetModification = $offsetModification;
        $this->limit = $limit;
    }
    public function modifiedOffset(): int
    {
        return $this->offset() + $this->offsetModification;
    }
    public function allUnlimited()
    {
        return $this->getFirstFromAllMatches();
    }
    public function all(): array
    {
        if ($this->limit === -1) {
            return parent::all();
        }
        return array_slice(parent::all(), 0, $this->limit);
    }
}

Uwagi

Naprawdę trudno mi było cokolwiek znaleźć w kodzie do czego mógłbym jakkolwiek się „przyczepić”. Właściwie to co mam to bardziej kwestie do przedyskutowania niż jakieś faktycznie mocne uwagi.

Po pierwsze zastanowiłbym się nad zmianą klasy ExceptionFactory na statyczną. Ponieważ zarówno ona, jak i klasa, z której korzysta czyli FailureIndicators, nie przechowują żadnego stanu to można by uniknąć za każdym razem tworzenia obiektu fabryki. Tym bardziej, że miejsce, w którym jest ta klasa używana i tak tworzy jej obiekt bezpośrednio. Więc nie ma mowy o możliwości podmiany implementacji na inną. A w tym momencie użycie ExceptionFactory wygląda tak:

return (new ExceptionFactory())->retrieveGlobals($this->methodName, $result);

Jednak jak mówiłem, jest to bardziej kwestia przedyskutowania za i przeciw niż zarzut co do jakości kodu.

Druga sprawa to klasa DelimiterParser, a konkretnie metoda getDelimiter():

    public function getDelimiter(string $pattern): ?string
    {
        if (strlen($pattern) < 2) {
            return null;
        }
        $firstLetter = $pattern[0];
        if ($this->isValidDelimiter($firstLetter)) {
            $lastOffset = strrpos($pattern, $firstLetter);
            if ($lastOffset === 0) {
                return null;
            }
            $flags = substr($pattern, $lastOffset);
            return $firstLetter;
        }
        return null;
    }

Odwracając drugi warunek można zmniejszyć ilość zagnieżdżonych bloków:

    public function getDelimiter(string $pattern): ?string
    {
        if (strlen($pattern) < 2) {
            return null;
        }
        $firstLetter = $pattern[0];

        if ($this->isValidDelimiter($firstLetter) === false) {
            return null;
        }

        $lastOffset = strrpos($pattern, $firstLetter);
        if ($lastOffset === 0) {
            return null;
        }
        $flags = substr($pattern, $lastOffset);
        return $firstLetter;
    }

Ale po raz kolejny nie jest to żaden poważny problem i spokojnie pierwotną wersję można uznać za „czystą”.

Parę słów o repozytorium

Samo repozytorium też jest warte poświęcenia mu kilku słów.

Pierwsza rzecz jaka od razu rzuca się w oczy to plik README. Zawiera on dokładny opis całego projektu i stanowi świetną dokumentację do niego. Znalazłem tam wszystko czego mógłbym szukać chcąc wykorzystać bibliotekę Daniela. Zastosowanie, przypadki użycia, sposób instalacji. Czego chcieć więcej? Wszystko opisane zwięźle i poprawnym angielskim.

Jeżeli ktoś się zastanawia jak się pisze takie pliki README to śpieszę z odpowiedzią – powstają z wykorzystaniem składni Markdown.

Druga sprawa to commity. Mają krótkie ale konkretne nazwy. Przeglądając ich listę wiadomo jakie pliki czy klasy były zmieniane. Wiadomo co się działo w kodzie. Żadnych commitów typu „fix”, „something”, „fix to fix”.

Znalazło się nawet miejsce dla Issues gdzie sam autor opisuje jakie funkcjonalności planuje dodać. Super!

Podsumowanie

Jestem w sumie zadowolony, że na początku trafiło się takie repozytorium. Bo mogę pokazać jak powinna wyglądać prezentacja projektu. Jeżeli zobaczyłbym takie repozytorium u osoby, która wysłała do nas CV to zdecydowanie chciałbym ją zaprosić na rozmowę. Mimo, że biblioteka jest niewielka to wszystko jest robione jak najbardziej profesjonalnie, schludnie i w przemyślany sposób.

Oby jak najwięcej takich repo na GitHubie!

Jeżeli widzisz, że coś przegapiłem albo uważasz, że jakiś fragment kodu napisałbyś inaczej to podziel się ze mną i autorem swoją opinią w komentarzu.