Aplikacja do oceny

Odpowiedz Nowy wątek
2019-08-31 09:23
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?

Pozostało 580 znaków

2019-08-31 10:58
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).

edytowany 3x, ostatnio: Kokoniłaj, 2019-08-31 11:00
Dziękuję za feedback :) NHibernate musiałem przypadkowo zainstalować, bo od początku zamierzałem korzystać z EF. Staram się metody private pisać z małej, natomiast protected/public z wielkiej. - Sumek QQ 2019-08-31 12:24

Pozostało 580 znaków

2019-08-31 13:31
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/Gy[...]re/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[...]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. ;)

edytowany 6x, ostatnio: nobody01, 2019-08-31 22:20

Pozostało 580 znaków

2019-08-31 13:49
0
        [Route("/cardio/{id}")]
        [HttpDelete]
        public async Task<ActionResult> Delete([FromBody] DeleteCardioResult command)
        {
            return await Remove(command);
        }

Po co ci to id w Route ?


Keep calm and blame frontend

Pozostało 580 znaków

2019-08-31 13:51
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. ;)

edytowany 3x, ostatnio: nobody01, 2019-08-31 14:44

Pozostało 580 znaków

2019-08-31 21:02
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

edytowany 6x, ostatnio: szydlak, 2019-08-31 21:57
Niemutowalne komendy bardzo fajne, wlasnie mam zamiar zaczac ich uzywac :) Co do pytania, to nie wiem. Ja nie umiec DDD, zaproponowalem niemutowalne kolekcje, bo autor chce go uzywac. :P Ale ogolnie przy DDD odczyty sie trudno robi, widzialem, jak ludzie pisali z palca SQLe w jakichs Readerach :) - nobody01 2019-08-31 21:18
@szydlak: A robiles tak jak w dokumentacji od MS? Wygooglaj 'implementing infrastracture persistence layer with ef core' - nobody01 2019-08-31 21:33
@nobody01: Teraz już nie pamiętam. Robiłem coś w tym stylu. Ale na pewno nie z tego linku. Pewnie sprawdzę za jakiś czas. thx - szydlak 2019-08-31 21:48
@szydlak Oczywiście middleware jest lepsze, ale to jeśli go wstrzykniesz do kontrollera bazowego to będziesz musiał zmienić każdy kontroller dziedziczący można obejść, robiąc jakieś brzydkie HttpContext.RequestServices.GetService<ILogger>(). Widziałem coś takiego tu: https://github.com/JasonGT/No[...]Controllers/BaseController.cs ;) - nobody01 2019-09-01 11:37

Pozostało 580 znaków

Odpowiedz
Liczba odpowiedzi na stronę

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