["Gra"] Czy mógłby ktoś sprawdzić kod?

Question

Dawno dawno temu Mixxy3k bawił się w programowanie aktualnie do tego powrócił. Głównym powodem nawrócenia jest chęć przeżycia w przyszłości mając pracę a nie jakieś 500+. Zatem zabrał się on za coś czego nigdy nie ukończył (i wciąż jest nie dokończone!) a mianowicie własna "prosta" 2D gierka w SFML. Niestety poniosła go fantazja i wpadł pomysł by wszystko było ładnie obiektowo a nawet były systemy wczytywania i zapisu oraz modyfikacja gry bez modyfikacji kodu! Wariat nie?

A teraz poważnie przez fakt że dawno się nie bawiłem w c++ boję się że wyleciało mi trochę za dużo. Nie chcę uwalić całego projektu przy początku. Redycja zawsze doprowadzała mnie do nerwicy i tak oto porzucałem projekt ZAWSZE. Dlatego odzywam się tutaj aby śmiałkowie którzy są w stanie zawitać ku norze abstrakcji i złego ochydnego kodu (a także języka angielskiego) mogli zobaczyć moje wypociny i ładnie mnie skrytykować.

Co jest "zrobione"

• Prosty menadżer do obsługi kochanej konsoli 
• Wyszukiwanie i ładowanie plików JSON
• Ładowanie tekstur i czcionki
• Pokazywanie okienka z napisem Loading (Wooo te emocje)

Chciało by się rzec tyle i aż tyle Tutaj je Github <3 

QNA

• Skończysz to kiedyś? Nie wiem ale cel jest tego nie porzucać dlatego piszę tutaj.
• Ma to komentarze? Jakieś ma ale boję się że za mało.
• Mogę podbierać kod i robić co zechcę? Jasne ale jak na nim zarobisz to stawiasz mi obiad i 20L coli waniliowej!

Answer 1

Hej. Zajrzałem w Twój kod. Mam kilka uwag. Jedne bardziej pomocne, inne mniej. :)

• pisze się "manager", nie "menager" :),
• po co stosujesz dynamiczną alokację dla typu GameState, który jest tylko typem wyliczeniowym? Zupełnie wystarczające byłoby umieszczenie tej zmiennej na stosie - już nie mówiąc o tym, że nigdzie nie dokonujesz dealokacji pamięci dla tej alokacji, czym tworzysz niewielki wyciek pamięci,
• dlaczego cała magia dzieje się w konstruktorze obiektu Engine? Konstruktor powinien służyć jedynie do skonstruowania obiektu - magia powinna powędrować do jakiejś metody doMagicStuff, którą wywołasz jawnie,
• do stanów - zamiast enuma - można zrobić prostą klasę State - będziesz mógł umieścić jakiś kod w konstruktorze i destruktorze takiego stanu, co będzie wykonywane automatycznie podczas zmiany stanu na inny (całkiem wygodne); zastosowanie enuma też jest w porządku, aczkolwiek jeśli Ci się projekt rozrośnie, to będziesz na niego przeklinał :),
• pisze się specification, nie specyfication :),
• fajnie byłoby rozdzielić logikę od rysowania,
• kolejny wyciek pamięci przy obiekcie buttons w klasie Menu,
• jak już masz podział na pliki .h i .cpp, rozdziel deklaracje i definicje,
• nie trzymaj zakomentowanego starego kodu (JsonMenager.cpp),
• JsonMenager::pathToString nie powinna być członkiem klasy JsonMenager - bo dlaczego jest? :),
• zapoznaj się z tzw. "rule of five",
• znów wyciek pamięci (logs z ConsoleMenager).

Generalnie nie ma wielkiej tragedii. ;) Pracuj dalej, powodzenia!

Comments

Wielkie dzięki za komentarz i poprawę moich anglosaskich błędów (Wincyj Google Translate Mixxy a nie z głowy!)
Aktualnie nie ma co rozdzielać(logika i rysowanie) bo nie ma co rysować. :3 Moja praca jest chaotyczna, dodaję to co jest potrzebne stąd chociażby klasa Menu(Która w zasadzie nie powinna istnieć..., jeszcze). Dzięki za podanie "rule of five" na pewno się przyda.
 

 

---

@draghan, Zmieniłem ConsoleManager wedle twoich zaleceń jednak wciąż nie jestem pewien czy wszystko tam jest dobrze. Mógł byś w wolnym czasie spojrzeć i napisać co i jak? 

Kod oczywiście na GitHub