3
\$\begingroup\$

As someone that started to learn basic python programming, I decided to make a Python tic-tac-toe game for the terminal, to develop my OOP skills.

I would like to know if my code has a lot of repetition and if it could have been done in a shorter, cleaner, faster, more efficient and more elegant way.

import re
import numpy as np
 
class Board_Game():
 def __init__(self):
 
 self.first_row = " 1 | 2 | 3"
 self.second_row = " 4 | 5 | 6"
 self.third_row = " 7 | 8 | 9"
 
 self.rows = [self.first_row,
 self.second_row,
 self.third_row ]
 
 self.markers = {'X': [],
 'O': []}
 
 self.positions = [1, 2, 3,
 4 ,5 ,6,
 7, 8, 9]
 self.avaliable_positions = [1, 2, 3,
 4 ,5 ,6,
 7, 8, 9]
 
 def print_board_coordinates(self):
 
 print('Board Coordinates:\n')
 print(" " + self.first_row)
 print(" ---|---|---")
 print(" " + self.second_row)
 print(" ---|---|---")
 print(" " + self.third_row)
 print('\n')
 def _render_row_state(self, row, marker):
 for k in marker.keys():
 for v in marker[k]:
 row = row.replace(str(v), k)
 row = re.sub(r'\d', ' ', row)
 
 return row
 def print_board(self):
 row_states = []
 for i , r in enumerate(self.rows):
 temp = self._render_row_state(r, self.markers)
 row_states.append(temp)
 
 print('Board:\n')
 print(" | | ")
 print(" " + row_states[0])
 print("----|---|----")
 print(" " + row_states[1])
 print("----|---|----")
 print(" " + row_states[2])
 print(" | | ")
 print('\n')
 def mark_board(self, marker, position):
 _valid_play = 0
 while _valid_play != 1:
 if marker not in ['X', 'O']:
 print('Wrong marker try again!')
 elif position not in self.positions or position not in self.avaliable_positions:
 position = int(input('Invalid move, choose another position\n'))
 else: 
 _valid_play = 1
 print("Valid play")
 self.markers[marker].append(position)
 self.avaliable_positions.remove(position)
 return position
class Player():
 
 def __init__(self, marker, first_play = False):
 
 assert marker in ['X', 'O'], f'Marker not allowed'
 
 self.marker = marker
 self._first_play = first_play
 self.owned_positions = []
 self.adversary_positions = []
 self.avaliable_positions = [1, 2, 3,
 4, 5, 6,
 7, 8, 9]
 self.winner = False
 def __repr__(self):
 return "Player()"
 def __str__(self):
 return f"Player {self.marker}"
 def make_move(self, position):
 mark = self.marker
 return mark, position
 def ack_own_position(self, position):
 self.owned_positions.append(position)
 def look_adversary_position(self, position):
 self.adversary_positions.append(position)
 def look_game_board(self, avaliable_positions):
 self.avaliable_positions = avaliable_positions
class Game():
 def __init__(self, players, board):
 self._players = players
 self._board = board
 self.winning_condition = [[1, 2, 3],
 [4, 5, 6], 
 [7, 8, 9], 
 [1, 4, 7],
 [2, 5, 8],
 [3, 6, 9],
 [1, 5, 9],
 [3, 5, 7]] 
 self.game_won = False
 self.play_count = 0
 self.game_winning_outcome = None
 def choose_first_player(self, first_player: int):
 """
 """
 self._players[first_player]._first_play = True
 self._first_player = first_player + 1
 self._player_time = first_player
 def _change_player_time(self):
 """
 """
 self._player_time = (self._player_time + 1) % 2
 self.play_count += 1
 def _check_winning_condition(self, player):
 """
 """
 for w in self.winning_condition:
 if set(w).issubset(player.owned_positions):
 player.winner = True
 print('Player', player.marker, 'has won')
 self.game_won = True
 self.game_winning_outcome = player.marker
 break
 if self.play_count == 8 and not self.game_won:
 self.game_winning_outcome = "Draw"
 print('DRAW!!!')
 
 def _play_game(self, position):
 """
 """
 
 _player = self._players[self._player_time]
 _other_player = self._players[(self._player_time + 1) % 2]
 _move = _player.make_move(position)
 _mark = _player.marker 
 
 # Actions
 new_position = self._board.mark_board(_mark, position)
 _player.ack_own_position(new_position)
 _other_player.look_adversary_position(new_position)
 self._check_winning_condition(_player)
# Main Game
# Game beggining who plays first and markers attribution
print('\nPlayer 1 choose between markers X or O')
game_markers = ['X', 'O']
marker_player_1 = input('')
dumb_player_count = 0
while marker_player_1 not in game_markers:
 print('You didnt chose a valid marker try again!')
 marker_player_1 = input('')
 dumb_player_count += 1
 if dumb_player_count == 2:
 exit('You are boring go play Tibia!')
player_1 = Player(marker_player_1)
marker_player_2 = [_ for _ in game_markers if _ != marker_player_1][0]
player_2 = Player(marker_player_2)
print(f'\nPlayer 1 marker: {player_1.marker}')
print(f'Player 2 marker: {player_2.marker}\n')
board = Board_Game()
game = Game([player_1, player_2], board)
print('Are you going to choose who plays first or should we chose randomly?')
_ = input('press R for randomly picking the first player:\n')
if _ in ['r', 'R']:
 idx = np.random.choice([0, 1])
 game.choose_first_player(idx) 
 print(f"\nPlayer {idx + 1} plays first")
else:
 _ = input('Press 1 if you want to Player 1 to start:\n')
 if _ == '1':
 game.choose_first_player(0) 
 print(f"Player 1 plays first")
 else:
 game.choose_first_player(1) 
 print(f"Player 2 plays first")
stop_game = False
while not stop_game:
 player = game._player_time + 1
 game._board.print_board_coordinates()
 game._board.print_board()
 move = int(input(f'Player {player} choose a position:\n'))
 game._play_game(move)
 game._change_player_time()
 if game.game_won == True or game.game_winning_outcome == 'Draw':
 stop_game = True 
game._board.print_board()
Toby Speight
87.3k14 gold badges104 silver badges322 bronze badges
asked Jul 1, 2021 at 12:22
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

I had a quick run through, those are just some of my notes

  1. I’d suggest following commonly agreed formatting, I.e. I don’t know if IDE can format arrays the way you have and it may get difficult to maintain.
  2. It would be nice to have all class properties defined at the top of the class with a type hint next to each one.
  3. print_board_coordinates and a board within print_board are duplicated, you can extract it out into a PrintBoard class and re-use
  4. Within mark_board, you might be able to get way with just position not in self.avaliable_positionswithinelif`
  5. You can add a class for Marker which will figure out for itself if market is allowed or not and then other classes don’t have to care about it.
  6. Some of the main code can also be extracted out into a Builder class, that handles setup of the game, and will make testing easier for you
  7. Not sure why player needs to know about available positions? I personally would have thought that owned_possitions would be sufficient.
  8. Given the number of lines, it would be nice to have draw.io diagram that outlines the flow of data in your application. Given the task, I think, communication should be layered with the game class being the mediator.
  9. I'd suggest typehinting everything it make it easier for reviewers, such as myself, to understand the subject quicker. Otherwise I have to scroll around and that takes energy.
answered Jul 4, 2021 at 7:39
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.