Ocena kodu c++ SFML

pytanie zadane 30 września 2015 w C i C++ przez sofnir Gaduła (4,690 p.)

Hej, napisałem swój pierwszy program w c++, przy użyciu biblioteki SFML - gra w tic tac toe.

Bardzo proszę o ocenę mojego kodu, co można zmienić, jakie błędy popełniam, co można zrobić prościej i krócej? :)

http://www19.zippyshare.com/v/1kKeof2k/file.html

6 odpowiedzi

odpowiedź 30 września 2015 przez event15 Szeryf (93,790 p.)

bool widocznosc1 = false;
bool widocznosc2 = false;
bool widocznosc3 = false;
bool widocznosc4 = false;
bool widocznosc5 = false;
bool widocznosc6 = false;
bool widocznosc7 = false;
bool widocznosc8 = false;
bool widocznosc9 = false;

A nie można zrobić tego jako tablicy boolowskiej typu widocznosc[9]? Tak samo z Aktywny?

void resetuj()

W kij za dużo odpowiedzialności:

najpierw pętla resetująca pola
ustawianie pól aktywnych
ustawianie widocznosci

Zamiast tego, zrób kilka funkcji, na przykład: zerujPola(), ustawAktywne(true albo false), ustawWidocznosc(true albo false)

Funkcja:

void sprawdz()

Znów zbyt wiele rzeczy na raz robi.

Funkcja główna programu jest również zbyt napchana. Powinno się utworzyć oddzielne funkcje dla każdej czynności.

Kurcze, tak dużo tutaj tego, że nawet do połowy nie doszedłem. Zadbaj o czytelność kodu. Serio.

PS. Ale obiecuję, że w wolnym czasie Twój program (oczywiście za Twoją zgodą) wykorzystam jako przykład refaktoringu gdzieś w jakimś artykule, bo to przykład idealny do takiego treningu :)

komentarz 1 października 2015 przez sofnir Gaduła (4,690 p.)

Spoko, nie ma sprawy, możesz wykorzystać :D (to chyba niezbyt dobrze o mnie świadczy, co? :D)

odpowiedź 30 września 2015 przez Patrycjerz Mędrzec (192,320 p.)

Więc tak:

Czy człowieku wiesz, że istnieje coś takiego, jak tablica? Zamiast tej sterty zmiennych z liczbami w nazwach mógłbyś stworzyć jedną, kompaktową tablice.
Zamiast flag odpowiedzialnych za wskazanie, jaka grafika jest widoczna, zrobiłbyś jeden kontener (np. std::vector) z tymi figurami, a rysowanie odbywałoby się za pomocą jednej pętli.
Radzę używać metody setFramerateLimit dla obiektu okna, ponieważ teraz program mi zżera dość sporo mocy procesora i także piszczy mi komputer (nie wiem, dlaczego).
Widzę od groma kodu, który się powtarza. Jestem pewien, że mógłbyś to zapakować w kilka funkcji, które zmniejszyłyby wielkość kodu.

To chyba wszystko. Gra nie jest zła, ale jest wiele rzeczy, które należy poprawić. Jak na początek, jest całkiem nieźle. Można robić gry z wieloma błędami, ale ważne, aby je robić.

Pozdrawiam!

PS: W pliku z grą brakuje pliku czcionki. Radziłbym poprawić ten problem...

komentarz 1 października 2015 przez sofnir Gaduła (4,690 p.)

Dzięki za porady, jasne że wiem, że istnieje tablica, wykorzystam to :)

odpowiedź 30 września 2015 przez niezalogowany

odpowiedź 3 października 2015 przez sofnir Gaduła (4,690 p.)

Witam ponownie, dokonałem poprawek kodu i zamieszczam wersję nr 2, do folderu dodałem również czcionkę.

http://www4.zippyshare.com/v/Ktbk077V/file.html

Bardzo proszę o ocenę, czy teraz jest lepiej i co jeszcze jest do poprawy? :)

komentarz 5 października 2015 przez efiku Szeryf (75,160 p.)

Warto poczytać i posłuchać opini innych: https://forum.pasja-informatyki.pl/65816/zabawa-w-refaktoryzacje-challange-forumowy?show=65817#a65817 ;)

odpowiedź 30 września 2015 przez Patryk Krajewski Nałogowiec (26,170 p.)

A nie możesz dać pliku .exe bo nie mam SFML a chciałbym zobaczyć?

komentarz 30 września 2015 przez sofnir Gaduła (4,690 p.)

Bardzo chętnie, ale nie potrafię tego zrobić, jeśli mógłbyś mnie poinstruować lub podesłać jakiś poradnik jak to zrobić to bardzo chętnie :D

komentarz 30 września 2015 przez niezalogowany

Daj exe źle, daj kod źle, nie da się wszystkim dogodzić :P

Btw, mi firefox blokuje (nie wiedzieć czemu) twój plik, więc może lepiej wrzuć na jakiegoś git'a, czy coś

komentarz 30 września 2015 przez Patrycjerz Mędrzec (192,320 p.)

Szymek, jeśli autor się nie obrazi, wysyłam ci kod: http://pastebin.com/fMhv1i8w.

komentarz 30 września 2015 przez niezalogowany

Jak już zobaczyłem, że wy sprawdzaliście to i ja postanowiłem na to zerknąć mimo tego że fierfox nie radził tego robić :P

komentarz 1 października 2015 przez sofnir Gaduła (4,690 p.)

Spoko, nie obraże się, to dla mnie miłe, że dajecie mi tyle cennych wskazówek :)

odpowiedź 1 października 2015 przez sofnir Gaduła (4,690 p.)

Mam do was takie pytanie, jak definiuje jakieś zmienne i funkcje, to w którym miejscu kodu je umieszczać? Jak tworze obiekty to gdzie je umieszczać? W pętli main? Czy wcześcniej? Wiecie, tak żeby było skeksownie wszystko xd

komentarz 1 października 2015 przez Patrycjerz Mędrzec (192,320 p.)

To wszystko zależy, w jakim miejscu są ci potrzebne. Tak naprawdę wszystko dąży do aspektu optymalizacji - wszystko wrzucone do maina będzie ciągle trzymane w pamięci, a tego przecież nie chcemy.

komentarz 1 października 2015 przez adrian17 Ekspert (345,160 p.)

Tak naprawdę wszystko dąży do aspektu optymalizacji

Nie, to się nazywa przedwczesna optymalizacja.

wszystko wrzucone do maina będzie ciągle trzymane w pamięci, a tego przecież nie chcemy.

A jak gdzieś indziej wrzucisz to nie będzie? Bo to zdanie ma trochę mało sensu. Masz rację że wszystko powinno być zadeklarowane tam gdzie jest potrzebne, to samo tyczy się pamięci.

komentarz 1 października 2015 przez Patrycjerz Mędrzec (192,320 p.)

Ja zaprezentowałem tylko jeden przykład, gdy wszystkie dane mamy zadeklarowane w funkcji main, nie mówiłem o innych przypadkach.

Zapomniałem wspomnieć, że ważną kwestią jest także czytelność i rozszerzalność kodu, która jest czasami ważniejsza, niż optymalizacja.