Ocena jakości kodu

Odpowiedz Nowy wątek
2016-03-23 12:23
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.

Pozostało 580 znaków

2016-03-23 12:34
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
z tym #pragma once to nie byłby z tym taki do przodu. W komercyjnych projektach w życiu nie widziałem go w użyciu. Z tego co słyszałem to są z tym problemy (szczegółów nie znam). - MarekR22 2016-03-23 16:18
@MarekR22 Chłopaki z Cloudius Systems dość intensywnie wykorzystują pragma once w swoich projektach. Zerknij na https://github.com/scylladb/scylla oraz https://github.com/scylladb/seastar - yurai 2016-03-23 18:30
nie twierdze, że nie ma takich projektów z użyciem #pramaga once, ale że jest ich mało. Z tego co doczytałem to powód pierwszy nieużywania #pramaga once jest to, że nie jest ono częścią standardu. Poza tym są jakieś problemy przy nietypowej organizacji katalogów, pojawiają się konflikty i #pragma once wtedy nie działa. - MarekR22 2016-03-24 10:37
@MarekR22 Ja z kolei nie twierdzę, że pragma once jest zajebista i mamy zawsze używać. Pragma once ma wady (które opisałeś) i zalety (takie jak potencjalne przyspieszenie kompilacji i mniej kodu preprocesora, który trzeba copy paste-ować między nagłówkami) jak wszystko. Podałem Ci tylko duże projekty w których się tego używa no bo napisałeś że jeszcze nie widziałeś. - yurai 2016-03-24 16:13

Pozostało 580 znaków

2016-03-23 12:41
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/Po[...]/blob/master/Menu.cpp#L16-L19 to się aż prosi o ?:
  7. https://github.com/ekicam2/Po[...]/blob/master/Menu.cpp#L41-L59 naprawdę nie czujesz, że coś tutaj jest nie tak?
  8. https://github.com/ekicam2/Po[...]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/Po[...]lob/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;
    };
  10. 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.

edytowany 1x, ostatnio: spartanPAGE, 2016-03-23 12:45
mogles zalinkowac tez projekt Age Of Demons https://bitbucket.org/mzawadzki91/puzzle4p/src :) - fasadin 2016-03-23 13:11

Pozostało 580 znaków

2016-03-23 16:15
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


Jeśli chcesz pomocy, NIE pisz na priva, ale zadaj dobre pytanie na forum.
edytowany 2x, ostatnio: MarekR22, 2016-03-23 16:28

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