Backend

Dlaczego logika w kontrolerach to zło?

24 października 2018 Marek Zając

Być może spotkałeś się w internecie ze stwierdzeniem, że nie powinieneś umieszczać logiki w kontrolerach. A jeśli się nie spotkałeś w internecie to spotkasz się przy pierwszej okazji, kiedy pokażesz komuś doświadczonemu kod swojej aplikacji webowej. O co tutaj chodzi? Dlaczego nie powinno się tej logiki pakować do kontrolera? Jakie są tego wady? Tego dowiesz się w dalszej części tekstu.

Marek Zając. Programista pracujący z .NETem i Angularem. Pracę zaczął od stażu w 2013 roku jednak samym programowaniem interesuje się od końca szkoły podstawowej. Aktualnie skupiony na tematach czystego kodu i architektury aplikacji. W pracy i poza nią chętnie dzieli się swoją wiedzą z innymi. Prowadzi bloga zajacmarek.com.

Spis treści

Logika – co to takiego?

Na początek warto sobie odpowiedzieć na pytanie czym jest ta logika, której nie powinniśmy umieszczać w kontrolerach? Bo każdy o niej mówi, a właściwie rzadko się to pojęcie wyjaśnia. Co dla początkującego może nie być najbardziej komfortową sytuacją.

Najbardziej zwięzłą definicją jaką mógłbym tutaj podać jest taka, że logika to wszystko to, co operuje na danych. Jeśli w jakiś sposób przetwarzasz dane to jest to logika. Taką logiką będzie więc walidacja danych, pobieranie danych z bazy, filtrowanie wyników, zmiana wartości w obiektach czy tworzenie tych obiektów.

Logiką nie będzie jedynie wywołanie ostatecznej funkcji uruchamiającej cały proces, np. funkcji „pobierz listę zamówień”. Bo mimo wszystko w końcu trzeba jakoś te dane do kontrolera dostarczyć. W tym celu mimo wszystko jakąś funkcję trzeba wywołać. Jednak dopiero ta funkcja powinna wykonywać jakiekolwiek operacje na danych, które być może nam finalnie zwróci.

Zobacz też

Backend Scala – fakty i mity. Prześwietlamy popularny język programowania

Trzy argumenty za logiką poza kontrolerem

Pokażę tutaj przykład z faktycznej aplikacji, którą zacząłem jakiś czas temu pisać. Jest to prototyp i w tym momencie akcja kontrolera zawiera właśnie w sobie logikę. Jest to akcja zwracająca pojedynczy wydatek zapisany w bazie danych:

        [HttpGet]
        [Authorize]
        [Route("{id}")]
        public EditExpenseDto GetSingle([FromQuery] string id)
        {
            var userId = Guid.Parse(User.Claims.First(x => x.Type == "UserId").Value);
            var expense = _context.Expenses.SingleOrDefault(x => x.Id == new Guid(id));

            if (expense == null || expense.UserId != userId)
            {
                Response.StatusCode = StatusCodes.Status400BadRequest;
                return null;
            }

            return new EditExpenseDto()
            {
                Identifier = expense.Id.ToString(),
                Store = expense.Store,
                Category = expense.Category,
                Date = expense.Date.Value.ToLongDateString()
            };
        }

Do przetestowania czy wszystko ma sens być może takie rozwiązanie by się sprawdziło. Jednak zobaczmy z czym wiąże się pozostawienie tej logiki w tym miejscu.

Zmiany w logice

Kod ma to do siebie, że się zmienia. Nawet w aplikacji, która ma określony cykl tworzenia i życia, a nawet konkretny i niezmienny zestaw funkcjonalności. Do pewnych rozwiązań programista bardzo często dochodzi iteracyjnie. Dopiero któraś z kolei próba sprawdzi się i będzie tym ostatecznym. Zdarza się też, że pewne zachowania są wspólne dla kilku obszarów i dopiero końcowy etap przetwarzania danych zmienia się. W takim wypadku zmiana w logice będzie się odbywać równolegle w dwóch miejscach.

Jeżeli zostawimy logikę w kontrolerze, tak jak w przykładzie powyżej, to w przypadku jakiejkolwiek zmiany musimy za każdym razem przeszukiwać wszystkie obszary, w których dany zestaw instrukcji może się znaleźć. Jeśli w powyższym przypadku zmieni mi się sposób tworzenia DTO albo dojdą nowe dane, to będę musiał to zmienić w każdym miejscu, gdzie pobieram pojedynczy wydatek. I jest spora szansa, że o którymś z tych miejsc zapomnę.

Rozwój aplikacji

Istnieje całkiem niemała grupa projektów, gdzie standardowo aplikacja zwraca swoje widoki użytkownikowi, ale daje też dostęp do API z tymi samymi danymi. Załóżmy więc, że mamy kontroler z logiką, który zwraca widok, typowa aplikacja w ASP.NET MVC. Wszystko sobie działa spokojnie i w pewnym momencie dochodzi wymaganie „dodajmy API dla zewnętrznych dostawców”. W sytuacji kiedy cała logika znajdowała by się w osobnych serwisach, repozytoriach i innych tego typu tworach to sprawa jest prosta. Dodajemy kontroler dla API i wywołujemy tą samą funkcję. W przypadku logiki w kontrolerze mamy dwie opcje – skopiować cały kod i mieć go w dwóch miejscach albo podjąć decyzję o refaktoryzacji i jednak przenieść go do osobnych klas. Pierwsze rozwiązanie odczujemy w przyszłości, kiedy trzeba będzie coś zmienić, a drugie spowoduje wydłużenie się developmentu w danym momencie. Oba w jakimś momencie powodują stratę czasu, a więc i pieniędzy.

Testowanie

Dobrą praktyką jest żeby jednak do kodu pisać testy. W powyższym przykładzie napisanie testów może i będzie możliwe, ale prawdopodobnie po pierwszym ich napisaniu już nigdy nie będziecie chcieli wracać w to miejsce.

Jeżeli aplikacja zwraca w akcjach widoki to nie przetestujesz po prostu wartości zwróconej przez nią tylko będziesz testować np. ActionResult, który zawiera mnóstwo dodatkowych danych. Jeżeli dodatkowo w kontrolerze korzystasz z jakichś filtrów czy atrybutów, które są zdefiniowane w innych klasach to tak naprawdę przetestowanie „gołych” akcji i tak nie daje Ci informacji jak obiekty się zachowają w trakcie działania. Testowanie kontrolera może też wiązać się z koniecznością mockowania wielu mechanizmów frameworka albo testowaniem obiektów tego frameworka takich jak ViewState czy ViewBag.

W konsekwencji spora część Twoich testów „jednostkowych” może okazać się tak naprawdę testami frameworka, który ktoś już przetestował albo testami integracyjnymi, które nie zawsze są najlepszym, pożądanym wyborem. Właściwie uważam, że testy akcji kontrolera, w których znajduje się logika zawsze będą testami integracyjnymi, bo tak naprawdę nigdy nie oddzielimy się od tej warstwy frameworka.

Co można zrobić?

Krótka odpowiedź – przenieść całą logikę do serwisów albo innych handlerów 🙂 Kod kontrolera w moim przypadku powinien wyglądać mniej więcej tak:

        [HttpGet]
        [Authorize]
        [Route("{id}")]
        public EditExpenseDto GetSingle([FromQuery] string id)
        {
            var userId = Guid.Parse(User.Claims.First(x => x.Type == "UserId").Value);
            return _expenseService.GetSingleExpence(userId, id);
        }

Jedynie pobranie id użytkownika bym zostawił jako element związany z samym frameworkiem.

A odpowiadając w bardziej rozbudowany sposób to powinniśmy wydzielić wszystko, co nie zależy bezpośrednio od kontrolera czy requestu do osobnych klas. Zależnie od potrzeb będą to serwisy, repozytoria, handlery czy jak to jeszcze nazwiemy. Chodzi o to, aby wszystko co się tam znajdzie mogło istnieć nie wiedząc w ogóle, że coś takiego jak kontroler istnieje.

Nawet jeżeli to, co mam na przykładzie z początku posta przeniósłbym 1:1 do funkcji w osobnej klasie, to pozbyłbym się problemów opisanych powyżej. Miałbym funkcję, którą mogę użyć zarówno w MVC, jak i API. Mógłbym dużo łatwiej całą logikę przetestować mockując jedynie bazę danych. A w przypadku zmian np. w polach DTO nie musiałbym się zastanawiać, czy ten fragment kodu nie jest przypadkiem używany w kilku miejscach, które powinienem zmodyfikować.