W tej części code review na chwilę zostawiam projekt pana kmph i przechodzimy do projektu Bartosza, który również wysłał mi swoje zgłoszenie. Tym razem mamy do czynienia z webową aplikacją pogodową pisaną w ASP.NET Web API. Autor poprosił aby ocenić jedynie API bez części frontendowej.

Link do GitHuba macie tutaj:

https://github.com/Zscfg/WeatherApp/tree/master/WeatherAppApi

Kod pokazywany w tym wpisie należy do jego autora. Pokazuję go tutaj dzięki zgłoszeniu się autora drogą mailową. Jeżeli chcesz aby Twój kod został oceniony zapoznaj się z TYM postem.

Tym razem jest okazja nie skupić się na kodzie na poziomie pisania klas czy metod ale spojrzeć na projekt szerzej. Przejdźmy więc do komentarza.

Wszędzie po trochu

Backend aplikacji podzielony jest na kilka projektów. To się chwali. W końcu porządek w kodzie i rozdzielenie pewnych elementów praktycznie zawsze ułatwiają zapoznanie się z projektem. Zacząłem więc przyglądać się poszczególnym strukturom projektów i wybranym plikom.

Przeglądając kod doszedłem do miejsca gdzie znalazłem DbContext Entity Frameworka. Wszystko ok. Siedzi to sobie w projekcie Infrastructure. Skoro mam context to gdzieś tu musi być model, na podstawie którego ten context działa z bazą danych. Jednak poszukiwania modeli zaprowadziły mnie do kolejnego projektu – Core. To w nim, w folderze EF siedzą encje domenowe.

Skoro znalazłem już model i context to pytanie gdzie są migracje? Na pewno nie ma ich w projektach Core i Infrastructure. Odnalazły się w głównym projekcie z API.

Co się tutaj stało?

Pierwszy i największy problem jaki się tutaj pojawia to rozrzucenie jednego elementu – dostępu do bazy – na 3 projekty. Powoduje to, że tak naprawdę żaden z tych projektów nie jest niezależny od pozostałych. Ciężko tutaj mówić o podziale. W gruncie rzeczy to wszystko to jest jeden niepodzielny projekt. W tym momencie chcąc zmienić framework albo po prostu jakoś zmodyfikować dostęp do bazy autor musi przeorać nie jeden, a co najmniej 3 projekty. W dodatku nie może tego zrobić po kawałku bo zmiana czegoś w jednym projekcie w kontekście Entity Frameworka automatycznie wymusza natychmiastową zmianę w pozostałych dwóch.

Drugi problem związany z takim rozwiązaniem, a być może nie taki oczywisty dla niektórych, to dodanie zależności od Entity Frameworka we wszystkich 3 projektach. Core zależy od EF, infrastruktura zależy od EF i API zależy od EF. I w przypadku aktualizacji musimy pamiętać żeby ją zrobić we wszystkich projektach.

Niby domena ale nie

Widzę, że Bartosz starał się tutaj wydzielić warstwę domeny, być może nawet poprowadzić projekt zgodnie z Domain Driven Design. Jednak coś tutaj nie do końca poszło w dobrą stronę.

Jeżeli Core miałbym traktować jako warstwę domeny to w żadnym wypadku nie powinna być zależna od ORMa. Taka zależność, w dodatku w wykonaniu, o którym trochę wyżej pisałem, kłóci się z ideą separacji warstw.

Druga sprawa, która mnie zastanawia i jest trochę związana z powtarzającym się tutaj tematem Entity Frameworka to foldery Domain i EF w projekcie Core. Z jednej strony w folderze Domain spodziewałbym się klas domenowych, a w folderze EF klas potrzebnych do kontaktu z bazą, a jednak tak nie jest.

Sytuacja wygląda w ten sposób, że w Domain widzę klasy, które są potrzebne dopiero w API albo infrastrukturze bo jest to np. klasa dla paginacji albo klasa rejestracji użytkownika. Za to w folderze EF są klasy, które ewidentnie są przygotowane dla Entity Frameworka, żeby mógł wygenerować te tabele w bazie i móc na nich operować poprawnie. Ale z drugiej strony te same klasy zawierają jakieś metody typowe dla klas domenowych, jakieś operacje na domenie czy typowo domenową walidację danych.

Nawet jeżeli nie miało być tutaj pisania kodu w podejściu DDD to chociażby same nazwy folderów są mylące i tak naprawdę ciężko powiedzieć o spójnym rozdzieleniu kodu. Skoro mamy warstwę infrastruktury to dlaczego klasy do paginacji są w Corze? A jeżeli encje zawierają też typowo domenowe elementy to dlaczego nie są w folderze Domain, który też istnieje? Itd.

Infrastruktura czyli wszystko

Ostatnia sprawa, raczej krótka i taka bardziej ogólna to projekt Infrastructure. Patrząc na niego i na to co zawiera mam wrażenie, że jest to po prostu worek na wszystko co nie pasowało do samego API albo do domeny. Z jednej strony mamy implementacje repozytoriów i serwisów i ogólnie dostępu do danych, a z drugiej strony mamy wszystkie używane tu i ówdzie heplery dla API pogodowych jak i konfiguracje IoC albo AutoMappera.

Moim zdaniem można, a nawet dobrze by było wydzielić tutaj przynajmniej 2 projekty. Przykładowo (nazwy robocze, żeby oddać ideę):

• Data – na wszystko co związane z dostępem do bazy danych i Entity Frameworkiem
• WeatherApi – czyli obsługa API pogodowych

Podsumowując

Jeśli chodzi o tematy zawarte w tym wpisie to polecam autorowi chociażby bloga Enterprise Craftsmanship. Wiedzy tam zawartej używamy chociażby w firmowych projektach od jakiegoś czasu i to się sprawdza.

Ogólnie muszę powiedzieć, że projekt, jeśli chodzi o strukturę i utrzymanie kodu, idzie w dobrym kierunku. Podoba mi się, że poza wspomnianymi tutaj wadami sprawia on wrażenie czystego i dobrze zarządzanego. Kontrolery, które nie zawierają logiki to jak miód na moje oczy! Nawet istnieją testy, także to już wyższy poziom :D

Bartoszowi życzę oczywiście powodzenia w dalszym rozwoju, a wszystkich Was zapraszam jak zwykle do dyskusji w komentarzach pod wpisem, albo na fanpagu na Facebooku.