Code Review apki pogodowej

Cześc, mógłby ktoś z doświadczonych programistów przejrzeć moja aplikacje? Powiedzieć co dobrze, co źle i co by poprawić. Aplikacja zrobiona w ASP.Net Core z frontem w Angularze oraz używa OpenWeather API. Link: https://github.com/AdrianGajewski1/Weather-App

lepiej nazywaj commity, nie add tylko added bo piszesz w czasie przeszlym

Nie podoba mi się, że Service zamiast zwracać dane to zwraca coś z warstwy prezentacji, czyli gotowy Response.

No i SQL z jedną tabelką na powiadomienia to gorzej niż z armatą do wróbla.

Saalin napisał(a):

Nie podoba mi się, że Service zamiast zwracać dane to zwraca coś z warstwy prezentacji, czyli gotowy Response.

No i SQL z jedną tabelką na powiadomienia to gorzej niż z armatą do wróbla.

To co według Ciebie mógłbym użyć do zapisywania powiadomień ? Gdzieś w pliku Json ?

To co według Ciebie mógłbym użyć do zapisywania powiadomień ? Gdzieś w pliku Json ?

RAMu.

Mnie ciekawi dlaczego NotificationsController zajmuje się tworzeniem powiadomień, dlaczego jest to nieskończona pętla i dlaczego jest tam thread.sleep.
To znaczy nie tyle ciekawi, że nie rozumiem idei tylko dlaczego np. to frontend nie odpyta serwera o dane. Piszesz o Signalu w komentarzu - ok. Ale póki co, to właśnie klient powinien odpalać tego typu zadanie.

Jaki jest sens wywoływania osobno metod GenerateUserID() i AddNotificationsAsync(). Ta pierwsza generuje guid, odpytuje bazę czy już takiego nie ma i jeśli jest to wali w pętli n zapytań. A potem znowu strzelasz do bazy insertem. Lepiej byłoby chyba to zrzucić w całości na bazę (włącznie z generowaniem identyfikatora) i od razu walnąć AddNotificationsAsync() - wtedy jeden strzał do bazy załatwia to co robiłeś minimalnie dwoma.

Zamiast takiej konstrukcji:

               if (await _dbContext.SaveChangesAsync() > 0)
                return true;

            return false;

wystarczy return _dbContext.SaveChangesAsync() > 0

Jak już tak namiętnie zamierzasz gwałcić bazę zapytaniami o to czy już gdzieś został wykorzystany GUID (w polu userid) to wypadałoby wpiąć tam chociaż jakiś indeks, który zaoszczędzi serwerowi trochę zasobów w hipotetycznej sytuacji kiedy w tej bazie będą miliony wpisów a api co chwila będzie generować zapytanie wymagające wykonania pełnego skanu tabeli.

EF z tego co pamiętam pozwala użyć w modelu bezpośrednio Guida