Code review - ASP.NET CORE

0

Piszę z prośbą o code review.
Aplikacja służy do zapisywania wydatków i prezentowania ich na wykresach. Najbardziej zależy mi na ocenie pod kątem zgodności ze wzorcem MVC, czy wszystko jest na swoim miejscu, clean code itp.

GitHub

1

Przeglądając na szybko. Poczytaj o Dependency injection i spróbuj zastosować w swoim projekcie, do tego przydadzą się interfejsy dla Twoich serwisów.

1

https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget.DataAccess/Models/HomeBudgetContext.cs#L32

Dla każdej encji wydzieliłbym config do osobnego .cs (IEntityTypeConfiguration), a później podpiął je przy pomocy

builder.ApplyConfigurationsFromAssembly

https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget/Controllers/ExpensesController.cs#L81

Nie lepiej byłoby wstrzykiwać te serwisy?

0

https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget.BusinessLogic/Searcher.cs#L12
W w wielu miejscach stosujesz prywatne zmienne zamiast property. Czemu takie podejście? Pytanie do innych - czy to jest ok?

0

https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget.DataAccess/Models/HomeBudgetContext.cs#L32

Dla każdej encji wydzieliłbym config do osobnego .cs (IEntityTypeConfiguration), a później podpiął je przy pomocy

builder.ApplyConfigurationsFromAssembly

Tutaj akurat użyłem podejścia database first i to zostało wygenerowane przez framework. Wiem, że powinienem użyć code first.

https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget/Controllers/ExpensesController.cs#L81

Nie lepiej byłoby wstrzykiwać te serwisy?

Myślę, że lepiej, ale w jaki sposób wówczas przekazać argumenty expenseId i userId?

https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget.BusinessLogic/Searcher.cs#L12
W w wielu miejscach stosujesz prywatne zmienne zamiast property. Czemu takie podejście? Pytanie do innych - czy to jest ok?

Zmienne stosuję wtedy, gdy używam ich tylko w danej klasie, jeśli potrzebuję użyć zmiennej na zewnątrz, wtedy stosuję właściwości.

3

Myślę, że lepiej, ale w jaki sposób wówczas przekazać argumenty expenseId i userId?

https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget.BusinessLogic/Services/ExpenseService.cs

Możesz zrobić, że ten ExpenseService dostaje wstrzyknięty HomeContext w ctorze, a ExpenseId i UserId przyjmuje jakaś metoda tego serwisu.

Przynajmniej ja bym tak zrobił :P

context.Expenses.Include("Category").Include("Unit")

dlaczego jako string, a nie lambda? lambda jest lepsza, bo potencjalny błąd będzie przy kompilacji, a nie runtime.

0

context.Expenses.Include("Category").Include("Unit")

dlaczego jako string, a nie lambda? lambda jest lepsza, bo potencjalny błąd będzie przy kompilacji, a nie runtime.

Gdzieś w necie znalazłem takie rozwiązanie, zaraz poprawię na lambdy.

0

Trochę to dziwne. Wygląda to tak, jakbyś nie chciał wykonać projekcji w jednym miejscu.

Dlaczego ta klasa tak się nazywa i dlaczego nie jest bezstanowa.?
https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget.BusinessLogic/CategoryMutator.cs

0
Gworys napisał(a):

Trochę to dziwne. Wygląda to tak, jakbyś nie chciał wykonać projekcji w jednym miejscu.

Gdzie taką projekcję powinienem wykonać?

Dlaczego ta klasa tak się nazywa i dlaczego nie jest bezstanowa.?
https://github.com/dawidKropkaD/HomeBudget/blob/master/HomeBudget.BusinessLogic/CategoryMutator.cs

Nie jest bezstanowa, bo staram się unikać klas i metod statycznych. Nazwałem ją tak, gdyż jest ona odpowiedzialna właśnie za mutację kategorii, a właściwie to za mutację nazwy kategorii.

0
panDawid napisał(a):

Nie jest bezstanowa, bo staram się unikać klas i metod statycznych.

Ojej, ale dlaczego?

0

Zwykle wzorce wykorzystuje się do zmniejszenia nadmiarowych instancji. Rozumiem, że chcesz postępować według wzorców, pisać dobry kod etc... Jeszcze kawał drogi przed tobą na początek dowiedź się jak działa ORM a szczególnie lazy loading i co to jest select n + 1. Najlepiej podłącz sobie slq profiler, kiedy używasz ORM.

0
somekind napisał(a):
panDawid napisał(a):

Nie jest bezstanowa, bo staram się unikać klas i metod statycznych.

Ojej, ale dlaczego?

Bo wolę programowanie obiektowe.

4
panDawid napisał(a):

Bo wolę programowanie obiektowe.

Programowanie obiektowe nie oznacza pisania kodu, który nie ma sensu, tworzenia instancji tam, gdzie nie mają one sensu, ani nieużywania static tam, gdzie ma to sens.

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