Aplikacja do oceny

1

Witam, chciałbym prosić o ocenę mojej aplikacji oraz wskazówki co w niej zmienić.
https://github.com/SumekQQ/Gym.Api
Aplikacja to prosty CRUD do odnotowywania swoich osiągnięć związanych ze sportami siłowymi. Możemy dodawać własne ćwiczenia, plany treningowe oraz oczywiście rezultaty.
Zdaje sobie sprawę że zastosowane w aplikacji patterny to przerost formy nad treścią i że szło to wszystko napisać o wiele prościej i szybciej :)

Aplikacje chciałbym dalej rozbudowywać w kierunku siłownianego social media, czyli że użytkownicy mogą się dodawać, oglądać swoje wyniki i ze sobą rywalizować etc, albo takiego systemu wspomagania pracy trenerów, czyli że mogą dodawać swoich podopiecznych i analizować ich wyniki. Pytanie tylko czy jestem w stanie dodać te nowe ficzery czy lepiej zacząć pisać od nowa?

2

Według mnie warto kontynuować dlatego, że w pracy programisty nie zawsze mamy tą przyjemność zaczynać projekt od nowa. Jak jeszcze nie pracujesz jako programista, a szukasz pracy to możesz dodać github do CV bo przebija on 95% projektów, które są umieszczane w dziale kariera. Chyba na początku chciałeś skorzystać z NHibernate bo w klasie Exercise została niepotrzebna dyrektywa z using. CSproj tez zawiera NHibernate. Pisanie metod private/protected z małej litery jest dla mnie dość dziwne. Zamiast IsExist w repozytoriach proponuję DoesExist() lub DoExist w zależności jaką liczbę elementów sprawdzamy.
Przejrzałem tylko powierzchownie ważniejsze (w mojej ocenie klasy).

0

Trudno się do czegoś przyczepić, widać, że robiłeś projekt na podstawie kursu Piotra Gankiewicza ;)
Co zauważyłem:

  1. Masz mutowalne kolekcje w domenie: https://github.com/SumekQQ/Gym.Api/blob/237a0df0c1d82852e4c0c6fd65caab749a40c729/Gym.Core/Models/TrainingPlan.cs#L13 Powinny być read-only
  2. Skoro już wstrzykujesz szynę do kontrolera, to dlaczego wstrzykujesz też serwisy? Nie możesz sobie wysyłać też zapytania, a nie tylko komendy?
  3. Skoro już masz handlery, to po co masz też serwisy? I dlaczego do każdego serwisu masz interfejs? Piotr już chyba sam stwierdził, że to nie ma sensu i w nowym projekcie zrobił tylko same handlery: https://github.com/devmentors/DNC-DShop.Services.Orders/blob/master/src/DShop.Services.Orders/Handlers/Orders/BrowseOrdersHandler.cs
  4. Rzucanie wyjątkami wprawdzie nie jest złe, ale warto też wiedzieć, że można zwracać Result/Either.
  5. Planujesz zrobić aplikację multijęzykową, że masz ErrorCodes?
  6. Co Ci daje testowanie tych serwisów jednostkowo, skoro prawie cała logika jest w encjach?
  7. Brakuje Ci chyba logowania błędów w BaseController, tzn. widzę jedynie catch(Exception ex) { return StatusCode(...) } EDIT: Lepiej pomyslec o jakims middleware, jak pisze @szydlak
  8. Albo nie widzę, albo brakuje Ci też chyba walidacji w warstwie aplikacji.
    Ale to raczej szczegóły, niektóre subiektywne. Ogólnie bardzo dobry projekt. ;)
0
        [Route("/cardio/{id}")]
        [HttpDelete]
        public async Task<ActionResult> Delete([FromBody] DeleteCardioResult command)
        {
            return await Remove(command);
        }

Po co ci to id w Route ?

0

Pewnie po to, żeby było RESTowo :D Jeśli już mamy id w ścieżce, to ja bym użył jakiegoś customowego bindera, np. https://github.com/billbogaiv/hybrid-model-binding, który by stworzył komendę na podstawie ciała i na podstawie ścieżki (np. przy PUT). Chociaż tu można po prostu dać FromRoute. ;)

1

Możesz jeszcze zrobić w komendach zamiast

public class DeleteExercise : ICommand
    {
        public Guid ExerciseId { get; set; }
    }

w ten sposób:

public class DeleteExercise : ICommand
    {
        [JsonConstructor]
         public DeleteExercise(Guid exerciseId)
        {
             ExerciseId = exerciseId;
        }
         public Guid ExerciseId { get;  }
    }

No i jak dla mnie dziwne podejście to obsługi błędów w controllerze. Jak dla mnie bardziej naturalne miejsce do tego to jest jakiś middleware.
Powiedzmy, że chcesz dodać jakiś logger który będzie logował exceptiony. I jeśli go wstrzykniesz do kontrollera bazowego to będziesz musiał zmienić każdy kontroller dziedziczący. Raczej nie tędy droga.
Kolejna rzecz to wyłapujesz exceptiony tylko te, które powstają po wywołaniu dispatchera. Ale możesz mieć też np przed wejściem do kontrollera. I wtedy już nie wyłapiesz tego.

No i te serwisy. Po co jak masz handlery? Równie dobrze mógłbyś pominąć dispatchera i uderzać do serwisów. Controllery dalej byłby czyste. Ten dispatcher nic Ci nie wnosi. Ja serwisu robie przy handlerach ttylko wtedy gdy mi się kod duplikuje w handlerach.

@nobody01
A próbowałeś na niemutowalnych kolekcjach robić includa? Bo mi się coś sypało

1 użytkowników online, w tym zalogowanych: 0, gości: 1