Hej, miałby ktoś czas aby ocenić kod mojej aplikacji? Jest to sklep online zrobiony z ASP.Net core i Angular 9. Aplikacja jeszcze nie skończona, zrobiłem już większość rzeczy dotyczących profilu użytkownika i chciałbym się dowiedzieć czy idę w dobrą strone :)
Link: https://github.com/AdrianGajewski1/MotoShop
A jeśli ktoś by miał chęci to mógłby przejrzeć mojego całego gita i ocenić znajdujące się tam projekty.
Link: https://github.com/AdrianGajewski1?tab=repositories
0
2
Na szybko przejrzałem kilka plików z .NET Core i mam kilka następujących uwag:
- adresy Twoich endpointów nie są zgodne z przyjętą konwencją REST'a (a przynajmniej nie z poziomem dojrzałości na poziomie drugim - https://devkr.pl/2018/04/10/restful-api-richardson-maturity-model). Chodzi o to, że to co chcesz zrobić z zasobem powinny definiować nagłówki HTTP (GET, POST, PUT, PATCH, DELETE) i tak przykładowo chcąc dodać reklamę powinniśmy strzelić w Twoim wypadku pod następujący adres POST'em:
api/advertisements, natomiast u Ciebie jest toapi/advertisements/add, więc równie dobrze mógłbyś strzelać pod wszystko GET'em u siebie - https://github.com/AdrianGajewski1/MotoShop/blob/master/Web/MotoShop.WebAPI/Controllers/AdvertisementsController.cs#L55. To samoapi/advertisements/getAllitp. - do assertion w testach polecam
FluentAssertion, dużo przyjemniej się z tym pisze - https://github.com/AdrianGajewski1/MotoShop/blob/master/Web/MotoShop.Tests/Services/AdvertisementServiceTests.cs#L28 - zawsze preferuję Fluent API, niż używanie Data Annotations bezpośrednio na encjach, przez co rozdziela się odpowiedzialność walidacji po stronie bazy, a modelem domenowym - https://github.com/AdrianGajewski1/MotoShop/blob/master/Web/MotoShop.Tests/Services/AdvertisementServiceTests.cs#L28
- publiczne settery w encji - https://github.com/AdrianGajewski1/MotoShop/blob/master/Web/MotoShop.Tests/Services/AdvertisementServiceTests.cs#L28. Zdecydowanie warto zastosować tutaj private set i ustawiać pola tylko i wyłącznie poprzez metody w encji, które zawierają wewnątrz siebie walidację obiektu, przez co to model sam w sobie zabezpiecza się przez tym, aby jego stan był zawsze poprawny. Poczytaj o enkapsulacji/hermetyzacji, bo w tej chwili każdy może przypisać dowolną wartość do Twojej encji i zrobić zapis do bazy danych.
- dlaczego IShopItemService masz zarejestrowane jako Scoped a nie Transient? https://github.com/AdrianGajewski1/MotoShop/blob/master/Web/MotoShop.WebAPI/Extensions/IServiceCollectionExtensions.cs#L56