search

MateuszKrasinski / Gomoku

Project
1 stars 0 forks source link

Uwagi #1

Closed MarcinCiura closed 4 years ago

MarcinCiura commented 4 years ago

Projekt całkiem nieźle się prezentuje. Jeśli AI działa, to po uwzględnieniu uwag i dopisaniu testów będzie po wszystkim.

  1. opis_Gomoku.md lepiej przenieść do README.md.
  2. W ai.py dodać pusty wiersz między importami a stałymi (czy pylintowi się podoba tak, jak jest teraz?)
  3. Magiczne stałe "black", "white" i "_" zastąpić przez EMPTY, BLACK, WHITE = range(3) (vide 2. rozdział Poradnika).
  4. Atrybut b klasy Game ma niezrozumiałą nazwę (czy pylint go łyknął?).
MateuszKrasinski commented 4 years ago

Zmiany zostały wprowadzone. Projekt gotowy do oceny.

MarcinCiura commented 4 years ago

Dziękuję za zmiany. Oto kolejna porcja uwag.

  1. Proszę przeczytać punkt o antywzorcach pętli z 3. rozdziału.
  2. Zbędne nawiasy.
  3. Wolę operator.itemgetter().
  4. Niedobre jest powtarzanie BOARD_SIZE = 15, BOARDSIZE = 15 itp. stałych w różnych modułach.
  5. Czy tutaj dałoby się przekazywać graczy z zewnątrz (4. rozdział)? Dzięki temu można by grać białymi albo czarnymi, a nawet we dwóch ludzkich graczy, prawda? Wolałbym dwie klasy: HumanPlayer i ComputerPlayer, dziedziczące co trzeba z PlayerBase. Ta druga mogłaby wciągnąć dotychczasową klasę AI, której tworzenie luzem w Game, oderwanej od gracza, który jej używa, jest nieładne.
  6. W Game do .played_moves i .last_move lepsze będzie collections.namedtuple.
  7. Albo spacje, albo przecinki.
  8. W main.py nie ma funkcji main(). Luźno lewitujący kod włożyć do funkcji. Zamiast if ...: continue; break lepiej if not ...: break.
  9. (BOARD_SIZE - 1) // 2 - 1, (BOARD_SIZE - 1) // 2 + 1.
  10. test_check_board_state.py przemianować na check_board_state_test.py i powyrzucać self.setUp() z metod.
MateuszKrasinski commented 4 years ago

Dziękuj za uwagi. Zmiany zostały wprowadzone. Stworzyłem klasy PlayerBase, AiPlayer, HumanPlayer i przeniosłem do modułu players.py , a moduł ai został do tego modułu wciągnięty.

MarcinCiura commented 4 years ago

Dziękuję za zmiany. Jesteśmy blisko zakończenia.

  1. Bez obciachu (koniec 2. rozdziału Poradnika).
  2. To się powtarza w testach. Zrobić z tego gdzieś malutką funkcję pomocniczą.
  3. Przeczytać punkt o skutkach ubocznych importowania modułów w 3. rozdziale.
  4. Tu też. Najszybszy sposób to przeniesienie screen = ... do main() i przekazywanie do tych 2 funkcji, które go potrzebują.
  5. Zbędne ().
  6. Zbędny pass. Sam docstring wystarcza. Jak Pan chce, można dać raise NotImplementedError.
  7. Proszę zauważyć, że to jest zupełnie nieużywane. Powinno być w game.py, gdzie .menu() powinno zwracać GameSettings(player1=..., player2=..., ...) zamiast krotki, w której indeksy są magiczne, i w main() używać settings.mode itp. (game_ jest zbędne) zamiast game_settings[3] itp.
MateuszKrasinski commented 4 years ago

[Stworzyłem funkcje pomocniczą w check_board_state.py i wywołuje ją w testach tego modułu i konstruktorze klasy Game. Moduł gui.py wciągnąłem w klase bazową Gui, gdzie przez konstruktor jest przekazywany screen utworzony w mainie. Pozostałem klasy, które znajdowały sie w tym module dziedzicą screen po klasie Gui. GameSettings zostało przeniosione do game.py i używane w momencie returna z .menu do maina. Dziękuje za uwagi.

MarcinCiura commented 4 years ago

Proszę jeszcze zerknąć na te trzy uwagi.

  1. Z tego wiersza wnoszę, że nie puszczał Pan ostatnio pylinta.
  2. Tu w Pythonie 3 wystarczy super().__init__(screen).
  3. Gdzieś chyba przepadło pygame.init().
MateuszKrasinski commented 4 years ago

Dziękuję za uwagi. 1.Pylint nie zgłasza zastrzeżeń co do tej linijki 2.Wszystkie miejsca w kodzie z super().__init zostały zamienione.

  1. pygame.init() zostało dodane na początku maina.
MarcinCiura commented 4 years ago

LGTM. Gratuluję pracowitości, jest Pan pierwszy.

MateuszKrasinski commented 4 years ago

Super! Dziękuję bardzo za rady :D