Code review - ASP.NET CORE

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

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.

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?

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?

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.

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.

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.

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

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.

panDawid napisał(a):

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

Ojej, ale dlaczego?

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.

somekind napisał(a):

panDawid napisał(a):

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

Ojej, ale dlaczego?

Bo wolę programowanie obiektowe.

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.