Ocena jakości kodu

0

Witam chciałbym dowiedzieć się czegoś o jakości kodu który udało mi się naklepać :)
tutaj link do repozytorium https://github.com/ekicam2/PoorGame

Wszelkie uwagi mile widziane.

1
  1. Za dużo się dzieje w main (brak klasy która by obsługiwała aplikacje
  2. Tworzenie menu / level na "stosie".
  3. Używanie gołych wskaźników. Od tego masz smart pointery (shared_ptr oraz unique_ptr)
  4. Zamiast IFDEF w nagłówkach lepiej używać #pragma once (juz praktycznie każdy kompilator to implementuje)
  5. Brak forward declaration
  6. Formatowanie np w player.hpp nie jest spójne.
  7. Używanie tablic (!). od teog masz std::vector
  8. Łamanie zasady DRY np w Menu::Init
  9. Magic numbery / stringi
3
  1. Brak jakiegokolwiek systemu budowania - gdyby ktoś chciał skompilować twój projekt, musiałby ręcznie wyszukać jakich zależności potrzebuje i tak samo ręcznie je podpiąć
  2. Struktura twojego projektu jest płaska - odczujesz to mocno gdy rozwiniesz projekt i pojawi się wiele, wiele plików.
  3. W wielu miejscach prosisz się o wycieki pamięci: m.i. https://github.com/ekicam2/PoorGame/blob/master/Player.hpp. Nigdy nie trzymaj surowych wskaźników! Swoją drogą SFML to biblioteka pisana w C++, więc nie musisz trzymać wszystkiego jako wskaźniki.
  4. https://github.com/ekicam2/PoorGame/blob/master/Player.cpp wtf? twój gracz nie ma definicji metod, znaczy to tyle, że nie można go nigdzie użyć.
  5. https://github.com/ekicam2/PoorGame/blob/master/Menu.cpp#L14 serio, std::vector i magiczne liczby oznaczające wielkość kontenera?
  6. https://github.com/ekicam2/PoorGame/blob/master/Menu.cpp#L16-L19 to się aż prosi o ?:
  7. https://github.com/ekicam2/PoorGame/blob/master/Menu.cpp#L41-L59 naprawdę nie czujesz, że coś tutaj jest nie tak?
  8. https://github.com/ekicam2/PoorGame/blob/master/Menu.cpp#L86-L100 cóż to za magiczny char?
    użyj do tego enum class direction{ up, down }; i raz jeszcze daruj sobie magiczne liczby
  9. https://github.com/ekicam2/PoorGame/blob/master/Menu.cpp#L102-L122 zamiast magicznego respond, które według swojej sygnatury powinno coś zwracać a tego nie robi, zbinduj sobie akcje z pozycjami w menu np tak:
struct menu_element {
  using action_t = std::function<void()>;
  std::string text;
  action_t action;
};
  1. Za dużo wymagasz od swoich scen. Rozbij to na mniejsze elementy.

Podsumowanie:
Niepotrzebnie pchasz wszędzie wskaźniki. Jeżeli chcesz już ich użyć, to zastosuj unique_ptr. Musisz też mocno popracować nad wydzielaniem odpowiedzialności. Popracuj też nad const-correctness swojego kodu.
Dobrze sobie zrobisz, jeśli rzucisz okiem na podstawowe wzorce projektowe używane w grach: http://gameprogrammingpatterns.com/contents.html

Sporo przed tobą pracy, ale jeśli będziesz sumiennie pracował, to będzie dobrze.

PS. Daruj sobie jakieś _prefiksy.

3

A ja pochwalę, bo może i wszystko powyżej jest prawdą, ale wiele ważnych projektów widziałem, które są w o wiele gorszym stanie.
Jeśli chodzi o styl to jest dobrze. Kod nie kole w oczy i pomimo rożnych niedoróbek można łatwo zrozumieć o co chodzi.
Jak na początkującego bardzo dobrze, będą z ciebie ludzie.

Z tego co mi się nie podoba: Player dziedziczy po sf::Sprite - dziedzicznie zdecydowanie tu nie pasuje

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