Chciałabym prosić was o ocenę mojego projektu. Jest to program do zarządzania planami z podziałem na plany dzienne, tygodniowe, miesięczne i roczne. Program jest obsługiwany w konsoli z wykorzystaniem biblioteki ncurses. Dodatkowo wykorzystuję bazę MySQL i boosta do obsługi dat. Za wszystkie uwagi bardzo dziękuję.
- klasa god w view ktora nazywa sie view
https://github.com/beti11/planner/blob/master/planner/views/sources/View.cpp
-
w view logika obslugi wejscia?
getMenuOptionFromUser
-
planService / sqlService, oba sa serwisami
Zapewne chodzilo Ci oserwis
ktory laczy sie z SQL. Ale to bardziej powinno byc ORM jak np libka tutaj (nie uzywalem wiec nie moge polecic, ale chodzi o przyklad) https://github.com/qicosmos/ormpp/blob/master/introduction_en.md
To, ze napisalas wlasny sqlService wielki +, ale zapewne nie bedziesz tego potrzebowac gdybys chciala to utrzymywac / rozwijac, bo takie rozwiazanie ma dosc duzo wad
Wiec tutaj zmienilbym nazwe przy sqlService bo to nie jest sqlService :D jezeli chcesz rozwinac projekt to na pewne zmiana na jakas libke ktora robi z parametryzowane zapytania by nie mozna bylo robic latwo sql injection -
czemu Facada robi jakis print?
void Facade::printMonthlyPlans()
-
formatowanie, nie jest zgodne z proponowanym przez C++, ale jako ze Twoj projekt to przynajmniej formatowanie jest jednolitey
-
Moze niektore dane mozna przeniesc do jsona? Albo jakis plik konfiguracyjny dla poczatku ktory ustawi wszystkie zmienne? To tylko taki pomysl
Ogolnie jest bardzo dobrze, jedynie co przeszkadza, to brak SOLID w klasach, jezeli ogarniesz bardziej single resposibility niz god object to bedzie super
-
PlanService - skoro calosc jest statyczna to przestanmy sobie mydlic oczy i zrob z tego namespace
Nie wiem jak inni ale mi osobiscie nie podobaja sie sciezki wzgledne w #include:
#include "../../plans/headers/DailyPlan.h"
Lepiej by bylo:
#include "DailyPlan.h"
plus ustawienie sciezek w cmake.
Ulatwia to refaktoring struktury katalogow.
- w tym samym pliku masz 2 antypatterny dotyczace catch:
- polykanie (pusta obsluga)
- catch/throw czyli nic-nierobienie
- Utworzenie obiektów
PlanService
iView
można przeprowadzić wewnątrz klasyFacade
Facade::Facade()
: planService { make_unique<PlanService >() }, view { make_unique<View >() } )
{}
.........
auto facade = make_unique<Facade>();
-
Zamiast używać
dynamic_pointer_cast
do rozpoznawania typu obiektu wFacade::checkPlanType()
pomyśl o wykorzystaniu polimorfizmu
i zaimplementowaniu wirtualnej funkcjiprintPlans()
dla każdego z typów planów. -
W pliku
SQLService.cpp
zauważyłem zbyt dużo powtarzającego się kodu w funkcjachreadAll****Plans( ... )
I może znowu lepszym rozwiązaniem będzie tutaj aby klasaPlan
dziedziczyła poSQLService
z wirtualną funkcjąreadAllPlans(...)
zaimplementowaną osobno dla każdego rodzaju planu.
Dziękuję bardzo wszystkim za ocenę kodu.
@fasadin
w view logika obslugi wejscia?
Chodziło mi o to, żeby umieścić w jednej klasie wszystko, co jest związane z biblioteką ncurses. Uważasz, że lepszym rozwiązaniem byłoby stworzenie drugiej klasy np. UserInput, która zawierałaby metody pobierające dane od użytkownika?
formatowanie, nie jest zgodne z proponowanym przez C++
W CLion jest opcja pozwalająca na wybranie formatowania o nazwie "Stroustrup" :) domyślam się, że pewnie o to chodzi, ale po zmianie CLion sugeruje zamianę wszystkich nazw zmiennych z camel case na format z podkreślnikiem. Której z tych dwóch wersji się używa zawodowo, bo ja do tej pory podczas nauki prawie zawsze w spotykałam się z camel case.
Chodziło mi o to, żeby umieścić w jednej klasie wszystko, co jest związane z biblioteką ncurses. Uważasz, że lepszym rozwiązaniem byłoby stworzenie drugiej klasy np. UserInput, która zawierałaby metody pobierające dane od użytkownika?
Tak.
A to View ma dokladnie wszystko, ale nie tylko co jest zwiazane z bibloteka ncurses, ale rowniez z wyswietlaniem samych danych.
Im mniejsze klasy ktore robia jedna konkretna rzecz, tym latwiej projekt sie utrzymuje
jezeli chodzi o formatowanie kodu polecam ten link
https://google.github.io/styleguide/cppguide.html