Prośba o ocenę kodu

Odpowiedz Nowy wątek
2019-07-22 20:50
0

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ę.

https://github.com/beti11/planner

Pozostało 580 znaków

2019-07-23 09:00
2

1) klasa god w view ktora nazywa sie view

https://github.com/beti11/pla[...]lanner/views/sources/View.cpp

2) w view logika obslugi wejscia? getMenuOptionFromUser

3) planService / sqlService, oba sa serwisami
Zapewne chodzilo Ci o serwis 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/o[...]lob/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

4) czemu Facada robi jakis print? void Facade::printMonthlyPlans()

5) formatowanie, nie jest zgodne z proponowanym przez C++, ale jako ze Twoj projekt to przynajmniej formatowanie jest jednolitey

6) 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

edytowany 2x, ostatnio: fasadin, 2019-07-23 09:01

Pozostało 580 znaków

2019-07-23 10:04
2

1) PlanService - skoro calosc jest statyczna to przestanmy sobie mydlic oczy i zrob z tego namespace

2) PlanService.cpp

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.

3) w tym samym pliku masz 2 antypatterny dotyczace catch:

  • polykanie (pusta obsluga)
  • catch/throw czyli nic-nierobienie

Szacuje się, że w Polsce brakuje 50 tys. programistów
edytowany 4x, ostatnio: vpiotr, 2019-07-23 14:02

Pozostało 580 znaków

2019-07-23 13:45
2

1) Utworzenie obiektów PlanService i View można przeprowadzić wewnątrz klasy Facade

Facade::Facade()
        : planService { make_unique<PlanService >() }, view { make_unique<View >() } )
{}
.........  
auto  facade = make_unique<Facade>();

2) Zamiast używać dynamic_pointer_cast do rozpoznawania typu obiektu w Facade::checkPlanType() pomyśl o wykorzystaniu polimorfizmu
i zaimplementowaniu wirtualnej funkcji printPlans() dla każdego z typów planów.

3) W pliku SQLService.cpp zauważyłem zbyt dużo powtarzającego się kodu w funkcjach readAll****Plans( ... )
I może znowu lepszym rozwiązaniem będzie tutaj aby klasa Plan dziedziczyła po SQLService z wirtualną funkcją readAllPlans(...) zaimplementowaną osobno dla każdego rodzaju planu.

Pozostało 580 znaków

2019-07-23 19:24
0

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.

Pozostało 580 znaków

2019-07-24 09:18
1

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

ok, dzięki :) - beti11 2019-07-24 09:30

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