Witajcie,

za enigmatycznym tytułem wątku kryje się moje pragnienie, by poznać Wasze spojrzenie na pewien problem w programowaniu. Temat jest dość długi, ale ciekawy, więc zachęcam do przeczytania. Leżałem sobie w łóżku i naszła mnie myśl, żeby o to Was zapytać (tak, czasem rozmyślam w łóżku o programowaniu - czy karetka już jedzie?).

Załóżmy, że mamy serwis z ogłoszeniami. Ludzie dodają ogłoszenia o dowolnej treści, a ogłoszenia te wiszą na liście od najnowszego. Każdy ma prawo odświeżyć ogłoszenie, czyli przenieść je znowu na górę listy, ale nie częściej raz na 24 godziny.

Nie jest to jak widać egzotyczny pomysł. Problem, o którym chcę porozmawiać, to sprawdzanie czy daną akcję da się wykonać i jak to elegancko zaprojektować. W naszej aplikacji z ogłoszeniami, kontrolka do odświeżania ogłoszenia pojawia się w wielu miejscach, każda ma zupełnie inną postać.

Problem polega na tym, żeby zdecydować który aktor aplikacji decyduje o tym czy ogłoszenia można odświeżyć. Dodatkowo pojawiają się pytania co robić w przypadku próby odświeżenia ogłoszenia, kiedy wcale nie powinno było się to odbywać.

Będę wrzucał przykładowy kod i po drodze zadawał pytania. Dla uproszczenia pomijam tu sprawę dobrych nawyków stylu kodu, takich jak używanie przestrzeni nazw, ORM itd.

[PHP] pobierz, plaintext 
class Announcement
{
    //
 
    public function renew(): void
    {
        /*
        wykonaj zapytanie:
        UPDATE `announcements`
           SET `renewed_at` = NOW()
         WHERE `id` = :id
        */
    }
}
[PHP] pobierz, plaintext 

Mamy odnawianie ogłoszenia w modelu Ogłoszenie, bez żadnego sprawdzania. Dodajemy kryterium:

[PHP] pobierz, plaintext 
public function renew(): bool
{
    if (time() - $this->getLastRenewedTimestamp() < 24 * 60 * 60) {
        return false;
    }
 
    /*
    wykonaj zapytanie:
    UPDATE `announcements`
       SET `renewed_at` = NOW()
     WHERE `id` = :id
    */
 
    return true;
}
[PHP] pobierz, plaintext 

W tym momencie nie da się odświeżyć ogłoszenia częściej niż raz na 24 godziny, ale nie ma możliwości sprawdzić czy ogłoszenie można odświeżyć, bez jego odświeżenia. Dodajmy więc metodę canBeRenewed():

[PHP] pobierz, plaintext 
public function canBeRenewed(): bool
{
    return time() - $this->getLastRenewedTimestamp() >= 24 * 60 * 60;
}
 
public function renew(): bool
{
    if (!$this->canBeRenewed()) {
        return false;
    }
 
    /*
    wykonaj zapytanie:
    UPDATE `announcements`
       SET `renewed_at` = NOW()
     WHERE `id` = :id
    */
 
    return true;
}
[PHP] pobierz, plaintext 

Teraz lepiej - możemy na przykład przekazać widokowi informację czy dane ogłoszenie może już być odnowione, czyli na przykład ukryć przycisk w sytuacji, gdy nie może. Wiem, że niektórzy programiści zatrzymują się na tym etapie, bo teoretycznie wszystko jest w porządku. Czy na pewno jednak zadanie sprawdzania możliwości wykonania odświeżenia powinno należeć do samego ogłoszenia? Mogą przecież wystąpić sytuacje wyjątkowe, kiedy na przykład administrator ma możliwość odświeżenia ogłoszenia nawet po minucie. Poza tym czy umieszczenie sprawdzenia w metodzie renew() nie przeczy zasadzie jednej odpowiedzialności? Usuńmy sprawdzenie, wracając do stanu początkowego:

[PHP] pobierz, plaintext 
public function renew(): void
{
    /*
    wykonaj zapytanie:
    UPDATE `announcements`
       SET `renewed_at` = NOW()
     WHERE `id` = :id
    */
}
[PHP] pobierz, plaintext 

Teraz nie ma z góry określonej zasady, że ogłoszenia nie można odświeżać w jakichkolwiek warunkach. Kto powinien się więc zajmować tym odświeżaniem? Moja pierwsza myśl to kontrolery. Powiedzmy, że mamy 30 ekranów z odświeżaniem ogłoszenia. Wiem - dużo na tym przykładzie, ale przy nieco zmodyfikowanym scenariuszu ta liczba mogłaby być już całkiem sensowna. Sprawdzenie możliwości odświeżania w każdej akcji zda egzamin, jednak wydaje się dość brzydkie, żmudne, łatwe do przeoczenia.

Ciekawostka: Spotkałem się z prawdziwymi aplikacjami, gdzie reguły formatu hasła były inne przy rejestracji i inne przy ustalaniu nowego hasła dla zapominalskich. Jest to identyczny problem!

Druga myśl to osobna klasa, OdświeżaczOgłoszeń. Nie za bardzo mam pomysł jak taka klasa mogłaby wyglądać, żeby obsłużyć różne scenariusze. Nawet nie wiem czy powinna to być jedna klasa, czy też różne scenariusze powinny być zawarte w różnych klasach Odświeżaczy. Dla skomplikowania przykładu powiedzmy, że ogłoszenia użytkowników premium mogą być odświeżane co godzinę. Zwykłe zwracanie prawdy lub fałszu w metodzie canBeRenewed() nie zdaje już egzaminu, bo lepiej wiedzieć dlaczego nie możemy odświeżyć ogłoszenia.

Kolejna kwestia to gdzie używać Wyjątków. Czy metoda renew() powinna wyrzucać Wyjątki przy próbie odświeżenia? Wydaje mi się, że nie - z tego samego powodu, z którego zrezygnowałem z użycia canBeRenewed() w metodzie renew(). Co innego w metodach klasy Odświeżaczy - tam już Wyjątki wydają się bardziej sensowne i możemy dzięki nim poznać przyczynę, dlaczego nie możemy odświeżyć:

[PHP] pobierz, plaintext 
class Renewer()
{
    public static function validate(Announcement $announcement): void
    {
        if ($announcement->user->isPremium() && time() - $this->getLastRenewedTimestamp() < 60 * 60) {
            throw new Exception('Premium users can renew every hour.');
        }
 
        if (!$announcement->user->isPremium() && time() - $this->getLastRenewedTimestamp() < 24 * 60 * 60) {
            throw new Exception('Standard users can renew once a day.');
        }
    }
 
    public static function check(Announcement $announcement): bool
    {
        try {
            self::validate($announcement);
        } catch (Exception $exception) {
            return false;
        }
 
        return true;
    }
 
    public static function renew(Announcement $announcement): void
    {
        self::validate($announcement);
 
        $announcement->renew();
    }
}
[PHP] pobierz, plaintext 

Póki co tyle. Na pewno jest kilka sposobów na zaprojektowanie tej funkcjonalności, ale ciekaw jestem, jak Wy to robicie. Podsumowując, pytania pojawiające się w tym temacie to:

1. Czy sprawdzanie możliwości wykonania danej czynności powinno odbywać się w samej metodzie wykonującej tę czynność?
2. Kiedy zwracać boolean, a kiedy wyrzucać Wyjątki?
3. Czy walidacja to zadanie jej przedmiotu, kontrolerów czy może osobnych klas, przeznaczonych tylko do tego celu?
4. Czy metoda na kształt check(Announcement $announcement), która tylko zwraca boolean na podstawie wystąpienia lub niewystąpienia Wyjątku to coś powszechnie stosowanego?
5. Jak zmieni się architektura pomysłu, gdy w określonych warunkach wywołanie akcji po prostu zepsuje nam obiekt? Chociażby usuwanie ogłoszenia - chcemy wiedzieć czy ogłoszenie można usunąć, bo albo wyświetlamy przycisk, albo nie. Ale ogłoszenia nie da się usunąć, gdy - dajmy na to - ktoś już na nie odpowiedział. Czy dobrym pomysłem jest zablokowanie tego z poziomu całej aplikacji przez brak metody, której jedynym zadaniem byłoby usuwanięcie ogłoszenia bez żadnego sprawdzenia?

Trochę czasu mi zajęło pisanie tego wątku. Mam nadzieję, że się wypowiecie Projektowanie klas to dla mnie najtrudniejsza (oczywiście obok wymyślania nazw!) strona programowania i czuję, że nie mam w tym jeszcze wystarczającej wprawy.

Może napisać klasę UserUpdateRequestManager.

Po polsku to by był MenedżerWnioskówOOdświeżenie.

Ta klasa by miała walidator pozwalania na update. Po pomyślnej weryfikacji egzekwowałaby odświeżenie.

Dalej nie piszę, bo byc może już błądzę?

Byłaby to bardziej ogólna wersja klasy Renewer. Być może jest to jakiś pomysł.

Dodam tylko, że sprawę dodatkowo komplikuje zawsze sama baza danych. Bo jeśli używamy ORM i przekazujemy modele, to te modele mogą być niepełne. Ktoś na przykład przy pobieraniu rekordu pominął kolumnę renewed_at i co wtedy? Albo przy zapisie modelu trzeba pamiętać, że ktoś po drodze mógł w tym modelu poustawiać wartości też innym kolumnom, których w tej metodzie przecież nie chcemy zapisać. Pisanie takich funkcjonalności zawsze mnie stresuje, bo jest w tym dużo pułapek. Być może jest to mój brak doświadczenia, a być może sama natura obiektowego programowania i bazy danych.

W Laravel są Policies, które są na sztywno połączone z konkretnym modelem (tutaj z Announcement).
Wydawałyby się idealnym miejscem do wydzielenia logiki prawa do konkretnego zasobu, ale na tym się właśnie kończy użyteczność.

[PHP] pobierz, plaintext 
Logged::can('renew', $announcement)
[PHP] pobierz, plaintext 

To pozwala wydzielić troszkę logiki związanej z autoryzacją i tym, że admin lub inny typ konta ma zawsze do tego prawo.
Minusem takiego rowiązania jest to, że to zwraca true/false lub wyrzuca reposne "Not authorized" i nie wyświetlisz zwrotnie konkretniejszego komunikatu.

Idąc jednak tym tokiem można by się pokusić o coś podobnego.
Modele raczej masz w całości tu przekazywane bo to wywołujesz zaraz na początku wywołania skryptu, tylko programista może popełnić błąd ale możesz sprawdzać obecność właściwości obiektu przed ich weryfikacją.