Conversation
SkySurferOne
left a comment
SkySurferOne
left a comment
There was a problem hiding this comment.
Generalnie widać, że masz problem z samym programowaniem obiektowym i z tym jak definiować klasy i ich zachowanie. Poczytaj sobie czym jest abstrakcja w programowaniu obiektowym i enkapsulacja. Warto też znać i stosować takie zasady jak SOLID, DRY, KISS itp. Jak tworzysz klasy to dobrze jakby były w osobnych plikach, które nazywają się tak jak klasa. Jeśli chodzi o styl pisania w samym pythonie to warto kierować się https://www.python.org/dev/peps/pep-0008/
|
|
||
|
|
||
| class TicTacToeGame: | ||
| class GameOn: |
There was a problem hiding this comment.
GameOn to niezbyt dobra nazwa dla klasy. Klasa powinna być rzeczownikiem i powinna być abstrakcją jakiejś rzeczy nie czynności. To metody opisują czynności wykonywane na obiekcie klasy. Dobrym przykładem klasy mogłaby być np. Game, Board, Player. Gdzie Game startuje nową grę i najprawdopodobniej jest singletonem, bo raczej nie chcemy tworzyć kliku gier na raz. Game tworzy planszę i dodaje do niej graczy.
There was a problem hiding this comment.
W tym przypadku zrobiłeś coś co się nazywa God object.
| @@ -39,45 +27,41 @@ def __init__(self): | |||
| '7': ' ', '8': ' ', '9': ' ' | |||
There was a problem hiding this comment.
Rozumiem, że to ma być plansza? Czemu nie jest to zwykła dwuwymiarowa tablica? Czy nie czytelniejsze jest board[0][1]? Wgle najlepiej jakby to była klasa, która przykrywa implementację planszy. Co jak nagle chciałbyś zrobić planszę 9x9? Musiałbyś zmieniać całą klasę GameOn? Swoją drogą tutaj naruszasz SRP. Board to też lepsza nazwa niż moves. I o ile mi wiadomo chociaż w pythonie nie ma czegoś takiego jak zmienne prywatne to stosuje się konwencję z podkreśleniami __foo https://stackoverflow.com/questions/1641219/does-python-have-private-variables-in-classes
| print('-----------') | ||
| print(f' 7 | 8 | 9 \n') | ||
|
|
||
| def current_battlefield(self): |
There was a problem hiding this comment.
Ta metoda powinna się zaleźć w klasie Board. Lepsza nazwa to np. show_board.
| print(f' 7 | 8 | 9 \n') | ||
|
|
||
| def current_battlefield(self): | ||
| print(f"\n {self.moves['1']} | {self.moves['2']} | {self.moves['3']} ") |
There was a problem hiding this comment.
jakby to była tablica wystarczyłoby zrobić podwójną pętlę lub jedną pętle z warunkiem. To by też pozwoliło Ci na zrobienie generycznego printa - działałby jeśli zmieniłby Ci się rozmiar planszy. Byłoby to też bardziej zwięzłe, bo powtarzasz tutaj kod - DRY.
| def play_round(self, player_symbol): | ||
| move = (input("Please select position for next move, from 1 to 9, don't use number " | ||
| "which is already taken: ")) | ||
| if move not in self.p_moves or len(move) != 1: |
There was a problem hiding this comment.
Ta logika powinna się znaleźć w klasie Board. Można by zrobić np. metodę set_sign(coord, player), która by rzucała exception gdyby coordynaty były złe. Można łapać wyjątek i logować błąd lub wystawić też metodę is_coord_valid()
There was a problem hiding this comment.
I wgle jest możliwe że warunek A faktycznie może być właśnie zdałem sobie sprawę z pomysłowości rozwiązania xD a co jeśli plansza byłaby rozmiaru 9x9? Dodałbyś liczby od 1 do 81? A co z liczbami z dwoma cyframi? Poza tym złożoność tej operacji jest liniowa. Można do zastąpić zwykłym porównaniem, które wykonamy w czasie stałym. - Przy tym rozwiązaniu wystarczy że zapamiętamy maksymalną wartość pola.move not in self.p_moves jest prawdziwy, a len(move) != 1 nie? Chyba nie?
| ['1', '4', '7'], ['2', '5', '8'], ['3', '6', '9'], | ||
| ['1', '5', '8'], ['3', '5', '7'] | ||
| ] | ||
| keys = [k for k, v in self.moves.items() if v == symbol] |
There was a problem hiding this comment.
Jakbyś wykorzystał tablicę nie musiałbyś robić takich rzeczy.
| print('Welcome to my game! Let\'s start with some rules.') | ||
| self.battlefield() | ||
|
|
||
| def win(self, symbol): |
There was a problem hiding this comment.
To nie jest dobra nazwa dla tej metody.
|
|
||
| if self.win(player_now): | ||
| print(f"Winner is player with '{player_now}' symbol!!!") | ||
| GameOff().start(True) |
There was a problem hiding this comment.
Przeczytaj to i zastanów się czy to jest zrozumiałe :D żeby podkreślić kuriozum te linijki napiszę co tu się dzieje: tworzysz nowy obiekt GameOff (wyłącz grę) na którym wykonujesz metodę START i podajesz parametr boolowski True (hmm a co jak podam False? Start zakończenia gry się nie wykona? Nie, nie zrestartujemy gry.)
|
|
||
| def game_off(self): | ||
| # tutaj może coś w stylu czy chcesz zagrać czy zobaczyć zasady najpierw? | ||
| class GameOff: |
There was a problem hiding this comment.
Ta logika powinna być w klasie Game.
| self.rules() | ||
| GameOff().start() | ||
| else: | ||
| GameOn().game() |
There was a problem hiding this comment.
To nie może tak być. Po prostu nie. :D
2 participants
Game is complete now - some testing needed in the future.