Ocena kodu, szukam mentora przy projekcie - .NET

0

Cześć,

Tak jak w tytule - uczę się od jakiegoś czasu .NET i jestem w trakcie tworzenia swojej pierwszej poważniejszej aplikacji webowej.
Poszukuję kogoś kto od czasu do czasu przejrzy mój kod, podpowie co jest nie tak, co można zrobić lepiej, podrzuci pomysł na dorzucenie jakiegoś wzorca projektowego, rozwieje czasem moje wątpliwości etc. Dodam, że jestem na poziomie "chciałbym zostać juniorem".

Projekt jest pod adresem:
https://github.com/mjxxx1/Car-Rental
Póki co za wiele nie mam, pobieram dane z DB, nieco je przetwarzam i wyświetlam listę samochodów, z opcją filtrowania (screen ja kto wygląda jest w README).
W zasadzie wszystko zaczyna się od tego kontrolera:
https://github.com/mjxxx1/Car-Rental/blob/master/CarRental/Controllers/CarController.cs
Kod działa, ale przed napisaniem testów chciałbym upewnić się, że jest choć na akceptowalnym poziomie.

stack: C#, .NET Core 2.1, MSSQL, Entity Framework Core, MediatR, testy chciałbym pisać w xUnit (jeszcze nie napisałem żadnego)

Oczywiście za poświęcony czas zapłacę.

2

To odpowiadając na pytania z kontrolera:

//Czy ten kontroler nie jest zbyt rozbudowany?

Jest.

//Czy można w jakiś sposób napisać test jednostkowy, który sprawdzi poprawność pobierania parametrów z widoku?

Pewnie można, ale nie warto raczej tracić czasu w obecnej wersji.
Ogólnie też nie warto raczej tracić czasu na jednostkowe testowanie kontrolerów, kontrolery nie powinny zawierać logiki i efektowniej jest po prostu testować API integracyjnie (wysyłając żądania HTTP).

//Mam wrażenie, że nie powienienem przechwytywać parametrów z widoku w ten sposób, ale nie mam pomysłu jak to zrobić prosćiej.``

Poczytaj o model binding.

Poza tym wszystko masz w jednym projekcie. Lepiej podziel solucję na projekty dla warstw (infrastructure, web, persistence, business logic - jakoś tak).

1

1.Że tak się spytam- skąd Ty w ogóle wytrzasnąłeś ten kod kontrolera ? Tego tak się nie robi w ASP.NET - poczytaj tak jak apisał wyżej Somekind o model binding. Ale ciekawi mnei skąð to wziąłeś , bo nie spotkałem jeszcze nigdzie tak karkołomnej konstrukcji - to trzeba się bardzo postarać żeby napisać pobieranie parametrów w ten sposób.
2. Czemu 2.1 , skoro jest LTS 3.1 a na horyzoncie powoli pojawia się 5 ?
3. Mediator - ogólnie Mediator to nienajgorsze podejście (kto co lubi) ale wydaje mi się że ja na początek wporwadza za dużo zawiłości które przysłonią ci pracę nad kodem. Sugeruję na razie klasyczny model interfejs-serwis - dependency injection.

3
  1. Dlaczego AdminController zawiera metodę służącą do pobierania listy pojazdów? Do tego powinieneś wykorzystać CarController i metodę List(). Przy jej wykonywaniu powinieneś oczywiście sprawdzić uprawnienia użytkownika. Do kontrolera Admin może dostać się teraz każdy, nie sprawdzasz uprawnień

  2. var connection = @"Server=MARIUSZ-PC\SQLEXPRESS;Database=Car_Rental;Trusted_Connection=True;" do przechowywania danych konfiguracyjnych lepiej wykorzystać plik appsettings.json, przekazywanie konfiguracji w odpowiednie miejsca też zostało już zaimplementowane i można to wykorzystać.

  3. dlaczego korzystasz z DbContext w trybie singleton?

  4. CarRental.Domain.Car możesz mi uwierzyć, ale wykorzystanie VIN jako klucza w bazie może się zemścić. Jest zwyczajnie zbyt długi. Użyj inta z autoinkrementacją i zostaw przypisywanie identyfikatora bazie danych. VIN obsłuż jako zwyczajną właściwość.
    Nie widać jeszcze samych rezerwacji, ale trzymanie właściwości IsInUse może być podatne na błędy - to trzeba za każdym razem obsłużyć. Dużo lepiej będzie dodać relację do rezerwacji (ewentualnie obsługi serwisowej i innych) i zwyczajnie sprawdzić na poziomie logiki biznesowej czy dany samochód jest w jakimś momencie wykorzystywany.

  5. po co ci tabelka zawierająca rodzaje paliwa i co gorsze relację 1:N z CarVersion? Tych rodzajów nie ma zbyt wiele, użyj enuma definiowanego w kodzie i będziesz miał 1 join mniej. Jeśli tak zrobisz to dobrze jest takowemu enumowi jawnie przypisać wartości liczbowe albo użyć konwersji do string w czasie zapisu do bazy (i odwrotnie w trakcie odczytu). Taki enum też nie powinien być modyfikowany bo to może rozwalić cały system.

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