Code review - ASP.NET CORE

Odpowiedz Nowy wątek
2019-05-03 19:45
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

Pozostało 580 znaków

2019-05-03 20:33
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.

Pozostało 580 znaków

2019-05-03 20:35
1

https://github.com/dawidKropk[...]dels/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/dawidKropk[...]ers/ExpensesController.cs#L81

Nie lepiej byłoby wstrzykiwać te serwisy?

edytowany 1x, ostatnio: WeiXiao, 2019-05-03 20:36

Pozostało 580 znaków

2019-05-03 20:40
0

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

edytowany 1x, ostatnio: Terrored, 2019-05-03 20:41

Pozostało 580 znaków

2019-05-03 21:01
0

https://github.com/dawidKropk[...]dels/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/dawidKropk[...]ers/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/dawidKropk[...]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.

edytowany 1x, ostatnio: panDawid, 2019-05-03 21:05

Pozostało 580 znaków

2019-05-03 21:12
3

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

https://github.com/dawidKropk[...]ic/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.

edytowany 12x, ostatnio: WeiXiao, 2019-05-03 21:27

Pozostało 580 znaków

2019-05-03 21:35
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.

Pozostało 580 znaków

dziś, 07:21
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/dawidKropk[...]inessLogic/CategoryMutator.cs


Unhandled Exception: System.MissingMethodException: Constructor on type 'System.Exception' not found.

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