Simple Python3 Checkers

Question

I'm a beginning "pythonist" my current task is to write a checkers game and I need some suggestions here and there:

• are the code style and literacy answering the criteria of python?
• are the modules of my program correlating with each other correctly?
• are the algorithms of finding possibilities (actions) for chechers working right? (just to add on to that one, did I properly set up the interaction of algorithms between themselves?)
• is the bot that I've made adjustable to that program?
• how can I make bot's algorithm more advanced?
• I don't really like the way I made game loop, so can you suggest any pattern that I could use?

Also, less important question, how can I improve the performance of the code?

Here is how it looks:

enter image description here

game_loop.py

import random
import deck_and_cheez
import bot
colors = ['○しろまる', '●くろまる']
deck = deck_and_cheez.Deck(random.choice(colors))
checker_pos = deck_and_cheez.CurrentChecker()
ALLIES = None
ENEMY = None
while True:
 print(f'Your color is: {deck.color}')
 deck.print_current_deck()
 if ALLIES is None:
 ALLIES = deck.color
 elif ENEMY is None:
 ENEMY = deck.color
 bott = bot.Bot(deck.deck, ENEMY)
 if deck.color == ALLIES:
 while True:
 checker = input('Choose you checker').upper()
 if deck.check_position(checker):
 if deck.check_checker_pos(checker):
 current_checker = checker_pos.coord(checker)
 move_coordinates = input('Enter coordinates').upper()
 if deck.move(move_coordinates, current_checker):
 deck.change_color()
 break
 elif not deck.move(move_coordinates, current_checker):
 continue
 elif deck.color == ENEMY:
 bott.move_bot()
 deck.deck = bott.deck
 deck.change_color()
 continue

deck_and_cheez.py:

class Deck:
 def __init__(self, color):
 """
 Function construct deck and store current information about checkers
 :param color: color of your checkers
 :rtype: str
 """
 self.deck = [[' ', '●くろまる', ' ', '●くろまる', ' ', '●くろまる', ' ', '●くろまる'],
 ['●くろまる', ' ', '●くろまる', ' ', '●くろまる', ' ', '●くろまる', ' '],
 [' ', '●くろまる', ' ', '●くろまる', ' ', '●くろまる', ' ', '●くろまる'],
 [' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '],
 [' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '],
 ['○しろまる', ' ', '○しろまる', ' ', '○しろまる', ' ', '○しろまる', ' '],
 [' ', '○しろまる', ' ', '○しろまる', ' ', '○しろまる', ' ', '○しろまる'],
 ['○しろまる', ' ', '○しろまる', ' ', '○しろまる', ' ', '○しろまる', ' ']]
 self.color = color
 self.w_checker = []
 self.b_checker = []
 self.queen_list_w = []
 self.queen_list_b = []
 def print_current_deck(self):
 """
 Function prints current deck
 """
 letters = ['A', 'B', 'C', 'D', 'E', 'F', 'G', 'H']
 numbers = ['1', '2', '3', '4', '5', '6', '7', '8']
 letter_count = 0
 print(f'\t {" ".join(numbers)}\n')
 for line in self.deck:
 a = '|'.join(line) # Разделить вертикальніми каждую яцеую и букві и ціфри добавить Можно джоинть по символу
 print(f'{letters[letter_count]}\t|{a}|\t{letters[letter_count]}')
 letter_count += 1
 print(f'\n\t {" ".join(numbers)}')
 def __coordinates(self, usr_inp):
 """
 Function user friendly input to computer
 :param usr_inp: Coordinate of cell
 :return: x and y coordinate
 :rtype: tuple
 """
 dict_pos = {'A': 0,
 'B': 1,
 'C': 2,
 'D': 3,
 'E': 4,
 'F': 5,
 'G': 6,
 'H': 7
 }
 if usr_inp[0] not in dict_pos.keys():
 return False
 x = dict_pos[usr_inp[0]]
 y = int(usr_inp[1])
 return x, y - 1
 def check_position(self, usr_inp):
 """
 Function checks whether coordinates are correct or not
 :param usr_inp: Coordinates
 :return: Coordinates
 :rtype: tuple
 """
 coordinates = self.__coordinates(usr_inp)
 if not self.__coordinates(usr_inp):
 print('Invalid letter')
 return False
 elif coordinates[0] < 0 or coordinates[1] < 0:
 print('Your coordinates is negative1')
 print('Please enter correct coordinate1')
 return False
 elif coordinates[0] > 8 or coordinates[1] > 8:
 print('Your coordinates out from scope2')
 print('Please enter correct coordinate2')
 return False
 return coordinates
 def calculate_possible_moves(self):
 """
 Function calculates white and black checkers and possible move for them
 :return:
 """
 self.w_checker.clear()
 self.b_checker.clear()
 for l_index, line in enumerate(self.deck):
 for e_index, element in enumerate(line):
 if element == '●くろまる':
 self.w_checker.append((l_index, e_index))
 elif element == '○しろまる':
 self.b_checker.append((l_index, e_index))
 move_checker_w = []
 move_checker_b = []
 move_checker_w.clear()
 move_checker_b.clear()
 for coordinate in self.w_checker:
 try:
 left_cor = self.deck[coordinate[0 ] +1][coordinate[1 ] +1]
 right_cor = self.deck[coordinate[0 ] +1][coordinate[1 ] -1]
 if left_cor == ' ':
 move_checker_w.append((coordinate[0 ] +1, coordinate[1 ] +1))
 if right_cor == ' ':
 move_checker_w.append((coordinate[0 ] +1, coordinate[1 ] -1))
 except IndexError:
 pass
 for coordinate in self.b_checker:
 try:
 left_cor = self.deck[coordinate[0] - 1][coordinate[1] - 1]
 right_cor = self.deck[coordinate[0] - 1][coordinate[1] + 1]
 if left_cor == ' ':
 move_checker_b.append((coordinate[0 ] -1, coordinate[1 ] -1))
 if right_cor == ' ':
 move_checker_b.append((coordinate[0 ] -1, coordinate[1 ] +1))
 except IndexError:
 pass
 def calculate_possible_move_for_check(self, usr_inp, cur_check):
 """
 Function calculates possible move for each checker
 :param usr_inp: Coordinate of move
 :param cur_check: Coordinate of checker
 :return: True or False
 :rtype: bool
 """
 if self.color == '●くろまる':
 if self.deck[cur_check[0]][cur_check[1]] == self.color:
 if usr_inp[0] == cur_check[0] + 1:
 if usr_inp[0] > cur_check[0] and sum(usr_inp) == sum(cur_check): # Left variant
 if self.check_cell(usr_inp):
 return False
 return True
 elif usr_inp[0] > cur_check[0] and sum(usr_inp) == sum(cur_check) + 2: # Right variant
 if self.check_cell(usr_inp):
 return False
 return True
 else:
 return False
 elif self.color == '○しろまる':
 if usr_inp[0] == cur_check[0] - 1:
 if usr_inp[0] < cur_check[0] and sum(usr_inp) == sum(cur_check) - 2: # Left variant
 if self.check_cell(usr_inp):
 return False
 return True
 elif usr_inp[0] < cur_check[0] and sum(usr_inp) == sum(cur_check): # Right variant
 if self.check_cell(usr_inp):
 return False
 return True
 else:
 return False
 def attack(self, usr_inp, cur_check):
 """
 Function describe attack
 :param usr_inp: Coordinate of move
 :param cur_check: Coordinate of checker
 :return: True or False
 :rtype: bool
 """
 u_x = usr_inp[0]
 u_y = usr_inp[1]
 c_x = cur_check[0]
 c_y = cur_check[1]
 try:
 if self.deck[u_x][u_y] != ' ' and self.deck[u_x][u_y] != self.color:
 if self.deck[u_x - 1][u_y + 1] == ' ': # Up right
 if self.deck[c_x - 1][c_y + 1] != ' ' and self.deck[c_x - 1][c_y + 1] != self.color:
 if u_x == c_x - 1 and u_y == c_y + 1:
 self.deck[c_x][c_y] = ' '
 self.deck[u_x][u_y] = ' '
 self.deck[u_x - 1][u_y + 1] = self.color
 return True
 except IndexError:
 pass
 try:
 if self.deck[u_x - 1][u_y - 1] == ' ': # Up left
 if self.deck[c_x - 1][c_y - 1] != ' ' and self.deck[c_x - 1][c_y - 1] != self.color:
 if u_x == c_x - 1 and u_y == c_y - 1:
 self.deck[c_x][c_y] = ' '
 self.deck[u_x][u_y] = ' '
 self.deck[u_x - 1][u_y - 1] = self.color
 return True
 except IndexError:
 pass
 try:
 if self.deck[u_x + 1][u_y - 1] == ' ': # Down left
 if self.deck[c_x + 1][c_y - 1] != ' ' and self.deck[c_x + 1][c_y - 1] != self.color:
 if u_x == c_x + 1 and u_y == c_y - 1:
 self.deck[c_x][c_y] = ' '
 self.deck[u_x][u_y] = ' '
 self.deck[u_x + 1][u_y - 1] = self.color
 return True
 except IndexError:
 pass
 try:
 if self.deck[u_x + 1][u_y + 1] == ' ': # Down right
 if self.deck[c_x + 1][c_y + 1] != ' ' and self.deck[c_x + 1][c_y + 1] != self.color:
 if u_x == c_x + 1 and u_y == c_y + 1:
 self.deck[c_x][c_y] = ' '
 self.deck[u_x][u_y] = ' '
 self.deck[u_x + 1][u_y + 1] = self.color
 return True
 except IndexError:
 pass
 else:
 print('Test Error')
 return False
 def move(self, usr_inp, cur_check):
 """
 Function describe move and this function main in module
 all magic starst from here
 :param usr_inp: String user friendly coordinate
 :param cur_check: Coordinate
 :return: True or False
 :rtype: bool
 """
 move_coordinates = tuple(self.check_position(usr_inp))
 if self.is_queen(cur_check):
 if self.color == '●くろまる' and cur_check in self.queen_list_w or self.color != '●くろまる' and cur_check in self.queen_list_b:
 print('You choose a Queen')
 if self.play_like_a_queen(move_coordinates, cur_check):
 return True
 return False
 self.calculate_possible_moves()
 if self.calculate_possible_move_for_check(move_coordinates, cur_check):
 if len(self.attack_list()) == 0:
 if not self.attack(move_coordinates, cur_check):
 self.deck[move_coordinates[0]][move_coordinates[1]] = self.color
 self.deck[cur_check[0]][cur_check[1]] = ' '
 return True
 elif len(self.attack_list()) > 0 and not self.attack(move_coordinates, cur_check):
 print('You must attack')
 return False
 while len(self.attack_list()) > 0:
 self.print_current_deck()
 print(self.color)
 self.move(input('Next attack').upper(), self.__calculate_new_cords(move_coordinates, cur_check))
 return True
 elif not self.calculate_possible_move_for_check(move_coordinates, cur_check):
 if len(self.attack_list()) > 0:
 if self.attack(move_coordinates, cur_check):
 return True
 print('You must attack')
 return False
 while len(self.attack_list()) > 0:
 self.print_current_deck()
 print(self.color)
 new_move = input('Next attack').upper()
 self.move(new_move, self.__calculate_new_cords(move_coordinates, cur_check))
 return True
 return False
 def check_checker_pos(self, usr_inp):
 """
 Function check if checker in cell
 :param usr_inp: Move coordinate
 :return: coordinate
 :rtype:tuple
 """
 coordinates = self.__coordinates(usr_inp)
 if self.deck[coordinates[0]][coordinates[1]] == '':
 print('Is no checker here')
 print('Please enter correct coordinate')
 return False
 return coordinates
 def change_color(self):
 """
 Function change color
 :return:
 """
 if self.color == '●くろまる':
 self.color = '○しろまる'
 else:
 self.color = '●くろまる'
 def check_cell(self, usr_inp):
 """
 Function check if cell is empty
 :param usr_inp: Move coordinate
 :return: True or False
 :rtype: bool
 """
 if self.deck[usr_inp[0]][usr_inp[1]] != ' ' and len(self.attack_list()) == 0:
 print('This cell is field')
 return True
 return False
 def can_attack(self, cur_check):
 """
 Function check if checker can attack
 :param cur_check: Checker coordinate
 :return: True or False
 :rtype: bool
 """
 x = cur_check[0]
 y = cur_check[1]
 try:
 if self.deck[x - 1][y - 1] != ' ' and self.deck[x - 1][y - 1] != self.color and self.deck[x - 2][y - 2] == ' '\
 and self.deck[x][y] == self.color and x - 1 >= 0 and y - 1 >= 0: # Left up
 return True
 except IndexError:
 pass
 try:
 if self.deck[x - 1][y + 1] != ' ' and self.deck[x - 1][y + 1] != self.color and self.deck[x - 2][y + 2] == ' '\
 and self.deck[x][y] == self.color and x - 1 >= 0 and y + 1 < 8: # Right up
 return True
 except IndexError:
 pass
 try:
 if self.deck[x + 1][y - 1] != ' ' and self.deck[x + 1][y - 1] != self.color and self.deck[x + 2][y - 2] == ' '\
 and self.deck[x][y] == self.color and x + 1 < 8 and y >= 0: # Down left
 return True
 except IndexError:
 pass
 try:
 if self.deck[x + 1][y + 1] != ' ' and self.deck[x + 1][y + 1] != self.color and self.deck[x + 2][y + 2] == ' '\
 and self.deck[x][y] == self.color and x + 1 < 8 and y + 1 < 8: #Down right
 return True
 except IndexError:
 pass
 return False
 def attack_list(self):
 """
 Function generate attack list
 :return: attack list
 :rtype: list
 """
 self.calculate_possible_moves()
 if self.color == '●くろまる':
 attack_list = []
 for coordinate in self.w_checker:
 if self.can_attack(coordinate):
 attack_list.append(coordinate)
 elif self.color == '○しろまる':
 attack_list = []
 for coordinate in self.b_checker:
 if self.can_attack(coordinate):
 attack_list.append(coordinate)
 return attack_list
 def __calculate_new_cords(self, usr_inp, cur_check):
 """
 Calculate new coordinate for current checker
 :param usr_inp: Move coordinate
 :param cur_check: Checker coordinate
 :return: x and y coordinate
 :rtype: tuple
 """
 u_x = usr_inp[0]
 u_y = usr_inp[1]
 c_x = cur_check[0]
 c_y = cur_check[1]
 a = -((u_x - c_x) * 2)
 b = -((u_y - c_y) * 2)
 x = c_x - a
 y = c_y - b
 return x, y
 def is_queen(self, cur_check):
 """
 Function check if checker is a queen
 :param cur_check: Current checker
 :return: True or false
 :rtype: bool
 """
 if self.color == '●くろまる' and cur_check[0] == 7:
 self.queen_list_w.append(cur_check)
 return True
 elif self.color == '○しろまる' and cur_check[0] == 0:
 self.queen_list_b.append(cur_check)
 return True
 return False
 def play_like_a_queen(self, usr_inp, cur_check):
 """
 Function describe queen logic
 :param usr_inp: Move coordinate
 :param cur_check: Checker coordinate
 :return: True or False
 :rtype: bool
 """
 u_x = usr_inp[0]
 u_y = usr_inp[1]
 c_x = cur_check[0]
 c_y = cur_check[1]
 if u_x != c_x or u_y != c_y:
 if self.deck[u_x][u_y] == self.color:
 print('You cant move self checker')
 elif self.deck[u_x][u_y] != self.color and self.deck[u_x][u_y] != ' ':
 try:
 if self.deck[u_x - 1][u_y - 1] == ' ': # Up Left
 self.deck[u_x - 1][u_y - 1] = self.color
 self.deck[u_x][u_y] = ' '
 return True
 except IndexError:
 pass
 try:
 if self.deck[u_x - 1][u_y + 1] == ' ': #Up Right
 self.deck[u_x - 1][u_y + 1] = self.color
 self.deck[u_x][u_y] = ' '
 return True
 except IndexError:
 pass
 try:
 if self.deck[u_x + 1][u_y - 1] == ' ': #Down left
 self.deck[u_x + 1][u_y - 1] = self.color
 self.deck[u_x][u_y] = ' '
 return True
 except IndexError:
 pass
 try:
 if self.deck[u_x + 1][u_y + 1] == ' ': # Down left
 self.deck[u_x + 1][u_y + 1] = self.color
 self.deck[u_x][u_y] = ' '
 return True
 except IndexError:
 pass
 return False
 else:
 self.deck[u_x][u_y] = '●くろまる'
 self.deck[c_x][c_y] = ' '
 return True
 def is_exit(self, usr_inp):
 """
 Function describe exit from input
 :param usr_inp: Some input
 :return: True or False
 :rtype: bool
 """
 if usr_inp == 'R':
 return True
class CurrentChecker:
 """
 Descibe current checker
 """
 def __init__(self, coordinates=None):
 """
 Construct current checker
 :param coordinates:
 """
 self.coordinates = coordinates
 def coord(self, usr_inp):
 """
 Function get correct coordinate
 :param usr_inp: Coordinate of checker
 :return: Coordinates
 :rtype: tuple
 """
 cords = Deck(1).check_position(usr_inp)
 self.coordinates = cords
 return self.coordinates

bot.py

class Bot:
 """
 Describe Bot
 """
 def __init__(self, deck, color):
 """
 Function construct bot
 :param deck: Current deck
 :param color: Bot color
 """
 self.deck = deck
 self.color = color
 self.checkers = []
 self.enemy_checkers = []
 self.moves = []
 self.queen_list = []
 def search_for_checker(self):
 """
 Function search all possible action for bot
 :return:
 """
 for l_index, line in enumerate(self.deck):
 for e_index, element in enumerate(line):
 if element == self.color:
 self.checkers.append((l_index, e_index))
 elif element != self.color and element != ' ':
 self.enemy_checkers.append((l_index, e_index))
 elif element == ' ':
 try:
 if self.color == '●くろまる':
 if self.deck[l_index - 1][e_index - 1] == self.color or self.deck[l_index - 1][e_index + 1] == self.color:
 self.moves.append((l_index, e_index))
 except IndexError:
 pass
 try:
 if self.color != '●くろまる':
 if self.deck[l_index + 1][e_index + 1] == self.color or self.deck[l_index + 1][e_index - 1] == self.color:
 self.moves.append((l_index, e_index))
 except IndexError:
 pass
 def move_bot(self):
 """
 Function describe bot move
 :return: Deck
 :rtype: list
 """
 self.is_queen_bot()
 if self.attack_like_queen():
 self.clears()
 return self.deck
 elif self.move_like_queen():
 self.clears()
 return self.deck
 elif self.can_attack():
 self.clears()
 return self.deck
 elif self.can_move():
 self.deck[self.can_move()[1][0]][self.can_move()[1][1]] = self.color
 self.deck[self.can_move()[0][0]][self.can_move()[0][1]] = ' '
 self.clears()
 return self.deck
 def can_move(self, checker=None):
 """
 Function check if bot can move
 :param checker: Checker coordinate
 :return: checker and move
 :rtype: tuple
 """
 self.search_for_checker()
 for checker in self.checkers:
 for move in self.moves:
 c_x = checker[0]
 c_y = checker[1]
 m_x = move[0]
 m_y = move[1]
 if self.color == '●くろまる':
 if c_x + 1 == m_x and c_y - 1 == m_y and self.deck[m_x][m_y] == ' ' or c_x + 1 == m_x and c_y + 1 == m_y and self.deck[m_x][m_y] == ' ':
 return checker, move
 elif self.color != '●くろまる':
 if c_x - 1 == m_x and c_y - 1 == m_y and self.deck[m_x][m_y] == ' ' or c_x - 1 == m_x and c_y + 1 == m_y and self.deck[m_x][m_y] == ' ':
 return checker, move
 def can_attack(self):
 """
 Function check if bot can attack
 :return: True or False
 :rtype: bool
 """
 self.search_for_checker()
 for checker in self.checkers:
 for enemy in self.enemy_checkers:
 if self.can_attack_more(checker, enemy):
 return True
 return False
 def is_queen_bot(self):
 """
 Function check check if checker is queen
 :return:
 """
 for checkers in self.checkers:
 if self.color == '●くろまる' and checkers[0] == 7:
 self.queen_list.append(checkers)
 elif self.color == '○しろまる' and checkers[0] == 0:
 self.queen_list.append(checkers)
 def move_like_queen(self):
 """
 Function describe logic of queen moving
 :return:
 """
 for queen in self.queen_list:
 for move in self.moves:
 q_x = queen[0]
 q_y = queen[1]
 m_x = move[0]
 m_y = move[1]
 if q_x != m_x or q_y != m_y:
 return queen, move
 def attack_like_queen(self):
 """
 Function describe logic of queen attack
 :return:
 """
 for queen in self.queen_list:
 for enemy in self.enemy_checkers:
 q_x = queen[0]
 q_y = queen[1]
 e_x = enemy[0]
 e_y = enemy[1]
 if q_x != e_x or q_y != e_y:
 try:
 if self.deck[((q_x - e_x)/2) - e_x][((q_y - e_y)/2) - e_y] == ' ':
 new_pos = (((q_x - e_x)/2) - e_x, ((q_y - e_y)/2) - e_y)
 self.deck[q_x][q_y] = ' '
 self.deck[e_x][e_y] = ' '
 self.deck[new_pos[0]][new_pos[1]] = self.color
 return queen, enemy, new_pos
 except IndexError:
 pass
 return False
 return False
 def clears(self):
 """
 Clear unused list
 :return:
 """
 self.checkers.clear()
 self.enemy_checkers.clear()
 self.moves.clear()
 def can_attack_more(self, checker, enemy):
 """
 Check if bot can attack again
 :param checker: Checker coordinate
 :param enemy: Enemy coordinate
 :return: True or False
 :rtype: bool
 """
 c_x = checker[0]
 c_y = checker[1]
 e_x = enemy[0]
 e_y = enemy[1]
 try:
 if c_x - e_x == 1 and c_y - e_y == 1 and self.deck[e_x + 1][e_y + 1] == ' ': # Up Left
 self.deck[c_x][c_y] = ' '
 self.deck[e_x][e_y] = ' '
 self.deck[e_x - 1][e_y - 1] = self.color
 for enemy in self.enemy_checkers:
 if self.can_attack_more((e_x - 1, e_y - 1), enemy):
 return True
 return True
 except IndexError:
 pass
 try:
 if c_x - e_x == 1 and c_y - e_y == -1 and self.deck[e_x - 1][e_y + 1] == ' ': # Up Right
 self.deck[c_x][c_y] = ' '
 self.deck[e_x][e_y] = ' '
 self.deck[e_x - 1][e_y + 1] = self.color
 for enemy in self.enemy_checkers:
 if self.can_attack_more((e_x - 1, e_y + 1), enemy):
 return True
 return True
 except IndexError:
 pass
 try:
 if c_x - e_x == -1 and c_y - e_y == 1 and self.deck[e_x - 1][e_y + 1] == ' ': # Down left
 self.deck[c_x][c_y] = ' '
 self.deck[e_x][e_y] = ' '
 self.deck[e_x + 1][e_y - 1] = self.color
 for enemy in self.enemy_checkers:
 if self.can_attack_more((e_x + 1, e_y - 1), enemy):
 return True
 return True
 elif c_x - e_x == -1 and c_y - e_y == -1 and self.deck[e_x - 1][e_y - 1] == ' ': # Down right
 self.deck[c_x][c_y] = ' '
 self.deck[e_x][e_y] = ' '
 self.deck[e_x + 1][e_y + 1] = self.color
 for enemy in self.enemy_checkers:
 if self.can_attack_more((e_x + 1, e_y + 1), enemy):
 return True
 return True
 except IndexError:
 pass

Answer 1

First, I like how you have everything spaced out instead of crammed together. I definitely prefer over-spacing to under-spacing. I think though, in a few places it's a little much. For example:

if usr_inp[0] not in dict_pos.keys():
 return False
x = dict_pos[usr_inp[0]]
y = int(usr_inp[1])
return x, y - 1

At some point, the spacing begins hurting readability. Once a function no longer fits on your screen all at once, you can no longer easily understand it in its entirety from a glance. Within functions, I like to at most have one empty line separating "parts". I also personally like the bodies of blocks to "hug" the "parent" of the block. For example, you have:

if self.attack_like_queen():
 self.clears()
 return self.deck
elif self.move_like_queen():
 self.clears()
 return self.deck

I would actually get rid of most of that space:

if self.attack_like_queen():
 self.clears()
 return self.deck
elif self.move_like_queen():
 self.clears()
 return self.deck

I like spacing, but I believe the indentation is enough to make each block distinct, and in this case, each line is simple enough that extra space around each line has little gain.

---

In move_bot, mentioned above, I see a few odd things.

First notice how every block ends in self.clears(); return self.deck. There's no point in duplicating that same code over and over. You also use several if...elifs to do the same thing. Just "connect" the conditions using or. I'd write this closer to:

def move_bot(self):
 self.is_queen_bot()
 can_move = self.can_move() # No point in needlessly calling this multiple times
 if can_move:
 self.deck[can_move[1][0]][can_move[1][1]] = self.color
 self.deck[can_move[0][0]][can_move[0][1]] = ' '
 if self.attack_like_queen() or self.move_like_queen() or self.can_attack() or can_move:
 self.clears()
 return self.deck

You could also nest the entire second block inside of the if can_move instead of checking can_move twice.

Major things to note here:

• Instead of calling self.can_move over and over, just call it once and save the result. If that function had any overhead, you're making your program needlessly slow.

• As mentioned before, all the previous separate conditions can be simple made into one longer one using or. This reads much nicer.

One other thing: the self.is_queen_bot() call at the top seems non-sensical. Why is a predicate (it starts with is_ which indicates that it's doing a check) being called, and the result is being ignored? The documentation for the function even says "Function check if checker is a queen". This is very confusing, because your function is not simply doing a check:

if self.color == '●くろまる' and cur_check[0] == 7:
 self.queen_list_w.append(cur_check) # Bam
 return True
elif self.color == '○しろまる' and cur_check[0] == 0:
 self.queen_list_b.append(cur_check) # And bam
 return True

It's mutating a list! This is incredibly unexpected, and definitely makes your program more difficult to understand. Naming and proper documentation are very important. They're how people unfamiliar with your code can understand what's going on. Neither the name nor the documentation suggests that the function is mutative. What if in the future you forget this yourself (because it isn't obvious), and try to call is_queen_bot to help with some new functionality that you're adding? The hidden side effects will cause unexpected stuff to happen in the background, and you'll need to debug it.

To fix this:

• Change the name to register_queen or something. The function is not returning a checked status to the caller, so it should not start with is.

• Correct the documentation to make it clearer what the intent of the function is.

---

You're using documentation-based typing. There's a cleaner way to tell the IDE and your users what types a function takes: type hints. For example, this function:

def check_position(self, usr_inp):
 """
 Function checks whether coordinates are correct or not
 :param usr_inp: Coordinates
 :return: Coordinates
 :rtype: tuple
 """

It isn't immediately clear what a "Coordinates" is. Presumably a tuple, but it could be some custom class or NamedTuple. I'd change it though to:

from typing import Tuple
Coordinates = Tuple[int, int] # Make a type alias that says a "coordinate" is a tuple of two integers
def check_position(self, usr_inp: Coordinates) -> Coordinates:
 """
 Function checks whether coordinates are correct or not
 """

Much cleaner. And, not only does this allow the IDE to know what a Coordinate, it allows your users to see it to. The less guessing and searching people have to do, the better.

In reality though, these hints are a lie, because your function actually returns False in many cases, and Coordinates in only one. This means it "optionally" returns Coordinates:

from typing import Tuple, Optional
def check_position(self, usr_inp: Coordinates) -> Optional[Coordinates]:
 """
 Function checks whether coordinates are correct or not
 """
 coordinates = self.__coordinates(usr_inp)
 if not coordinates: # Again, used the saved data here instead
 print('Invalid letter')
 return None # And return None instead of False to adhere to Optional
 elif coordinates[0] < 0 or coordinates[1] < 0:
 print('Your coordinates is negative1')
 print('Please enter correct coordinate1')
 return None
 elif coordinates[0] > 8 or coordinates[1] > 8:
 print('Your coordinates out from scope2')
 print('Please enter correct coordinate2')
 return None
 else:
 return coordinates # I put this in an else because I personally prefer that, even if it's implied anyway

---

In a couple places, you have:

except IndexError:
 pass

I can not overstate how bad this is. Do not do this. Can you guarantee, with 100% certainty, that never in the future will you accidentally typo code in there that will cause an IndexError unintentionally? You will make mistakes at some point, and throwing away exceptions like you are here will make your life miserable (I know, I've been there). I don't think I've ever run into a case where it's appropriate to catch an IndexError. I'm not saying there are no legitimate cases, but they're rare, and I don't believe that this is a good case here.

You appear to be catching them in case a check goes out of bounds off the board. Now, Python preaches an "It's easier to ask forgiveness than permission" ideology, but that doesn't mean that that thinking should be used in every case blindly. I would strongly encourage you do alter this code to first do a check to see the if the index is in bounds, then do the indexing. Indexing errors often appear as bugs, and again, you do not want to silence errors that will help you track down bugs. You'll find that your program will slowly begin acting more broken the more changes you make, but you won't get any errors indicating that anythings wrong. This is an awful place to be in, and I wouldn't wish that on you.

---

Instead of writing '●くろまる' and '○しろまる' all over the code, I'd save these some in a variable and use the variable instead:

BLACK_PIECE = '●くろまる'
WHITE_PIECE = '○しろまる'
. . .
if self.color == BLACK_PIECE and checkers[0] == 7:
 self.queen_list.append(checkers)
elif self.color == WHITE_PIECE and checkers[0] == 0:

Two major reasons:

• First, '●くろまる' and '○しろまる' are very close to what are known as "Magic Numbers". No, they aren't numbers, but they're conceptually similar. '●くろまる' and '○しろまる' in code have little inherent meaning. You may be able to guess what they're for, but it may not be obvious is every context. BLACK_PIECE and WHITE_PIECE however are easy to understand labels that immediately tell the reader what they're for.

• Second, pretend in the future you want to change what the pieces look like. What will be easier? Reassigning BLACK_PIECE and WHITE_PIECE to new characters, or altering every instance of '●くろまる' and '○しろまる' in your code. The former will definitely be simpler and leave less room for bugs to happen.

---

---

I'd keep going, but honestly, I just got home from work and my brain's fried.

Good luck!

Answer 2

Welcome to CodeReview! I'll start by saying thank you for posting a somewhat-complex package of code. At first blush, your code appears to be written in the generally-accepted style and appears well organized and somewhat documented.

Functionality

Before I actually review the code, I'll point out that I tried to play the game. Here's my experience:

Your color is: ○しろまる
 1 2 3 4 5 6 7 8
A | |●くろまる| |●くろまる| |●くろまる| |●くろまる| A
B |●くろまる| |●くろまる| |●くろまる| |●くろまる| | B
C | |●くろまる| |●くろまる| |●くろまる| |●くろまる| C
D | | | | | | | | | D
E | | | | | | | | | E
F |○しろまる| |○しろまる| |○しろまる| |○しろまる| | F
G | |○しろまる| |○しろまる| |○しろまる| |○しろまる| G
H |○しろまる| |○しろまる| |○しろまる| |○しろまる| | H
 1 2 3 4 5 6 7 8
Choose you checkerf1
Enter coordinatese1
Choose you checkerf1
Enter coordinatese2
Test Error
Your color is: ●くろまる
 1 2 3 4 5 6 7 8
A | |●くろまる| |●くろまる| |●くろまる| |●くろまる| A
B |●くろまる| |●くろまる| |●くろまる| |●くろまる| | B
C | |●くろまる| |●くろまる| |●くろまる| |●くろまる| C
D | | | | | | | | | D
E | |○しろまる| | | | | | | E
F | | |○しろまる| |○しろまる| |○しろまる| | F
G | |○しろまる| |○しろまる| |○しろまる| |○しろまる| G
H |○しろまる| |○しろまる| |○しろまる| |○しろまる| | H
 1 2 3 4 5 6 7 8

So I have some comments on the actual operation of the game :-).

Playability

First, the game board does not color black/white squares. This makes it harder to play, since the alternating colors help with distinguishing different rows/columns, as well as providing a visual crutch for understanding move possibilities.

Next, the coordinate system is perfectly functional, but not at all intuitive. I'd suggest you consider some alternatives, like labelling the individual checkers with letters A-L instead of circles. Similarly, you might consider enumerating the possible destinations for movement. Either explicitly (redraw the board with 1..4 markers) or implicitly (draw a compass rose with 1..4 on it alongside the board).

Input/Output

The prompt needs to include a space at the end. Otherwise, you get what I got:

Choose you checkerf1

There is no indication of a help system. If someone doesn't know how to play checkers, or doesn't remember, how do they get help? Where are the rules of play?

If I enter a bad move, as I did, there's no rebuff message. Instead, just a new prompt. You should explain that my move is invalid, and either print a rule summary ("You must move 1 space diagonally, or jump over an enemy piece diagonally") or print an enumeration of valid moves for the piece.

I don't know what a Test Error is. But I'm pretty sure that it shouldn't be appearing during gameplay.

Naming

I have two comments on your file naming. First, instead of game_loop.py you should have named the file checkers.py or main.py (or __main__.py). Because there's nothing obvious about what game this is, just looking at the file names. I mean, deck_and_cheez.py? What game did you start writing before you switched to checkers?

Second, there just isn't that much code in these files:

aghast@laptop:~/Code/so/checkers$ wc -l *py
 312 bot.py
 692 deck_and_cheez.py
 58 game_loop.py
 1062 total

Why not just move all the code into checkers.py? This isn't java, there's no requirement to have a bunch of little files laying around.

game_loop.py

Structure

Everything in this file should be in a function. Possibly a different function in a different class, but definitely in a function. The standard idiom for python scripts is this:

def main():
 ... all the stuff that isn't in a function ...
if __name__ == '__main__':
 main()

Use this idiom. It makes it possible to do a bunch of things, including writing unit tests, that will benefit you. It doesn't cost much (just one line, and some tabs), and pays off quickly.

Modularity

What do I learn from this code? (I deleted some vertical space for convenience.)

colors = ['○しろまる', '●くろまる']
deck = deck_and_cheez.Deck(random.choice(colors))
checker_pos = deck_and_cheez.CurrentChecker()
ALLIES = None
ENEMY = None
while True:
 print(f'Your color is: {deck.color}')
 deck.print_current_deck()
 if ALLIES is None:
 ALLIES = deck.color
 elif ENEMY is None:

Three things. First, is that your deck_and_cheez.Deck class is broken. Second, your deck_and_cheez.CurrentChecker class is even worse! Third, you aren't taking a broad enough view.

class Deck

A class is supposed to be self-sufficient. If you give it the required arguments at creation time, the returned instance will stand alone.

Let's look:

• Naming: checkers is a "board game". A better name for Deck would be GameState or Board. In English, a deck is either a floor on a ship, or an alias for a pack of cards. Poker and Pinochle are played with a Deck, while Checkers and Chess are played with a Board.

• From the colors = [...] variable, you don't have a Deck.COLORS that provides this data to callers. Yet, this is a pretty important part of the Deck so why isn't it there?

• From the Deck(random.choice(colors)), it seems you don't need to tell Deck what both colors are (you only pass in one color). Thus, I sense there is a second copy of the colors = [...] over in the Deck class somewhere. (In fact, it's worse. See below.)

• The code to set ALLIES and ENEMY is determining the value of the random choice you passed in as a parameter. And the two "constants" are only used to determine whose turn it is to play. This could be a part of Deck. It also could be implemented in code, just by writing player_turn() ; computer_turn().

Suggestions

I don't think you need to "specify" a player color on creation of a new Deck. I think you should just randomly pick one, and make it available to users of the class:

deck = Deck()
player_color, bot_color = deck.player_colors

Once you have the colors allocated inside Deck, you can write a method that cycles the player-turn tracking without having to pass in any parameters:

deck.end_turn()

You should provide a mechanism for determining the end of the game. That could be a method on Deck or an exception raised by the turn handlers. Doing this makes the game loop clearer.

while deck.in_progress:

or

try:
 while True:
 player_turn()
 robot_turn()
except GameOver:
 pass

class CurrentChecker

This class is so low-key that I almost didn't catch it. Your usage model is that you create an instance of the class:

checker_pos = deck_and_cheez.CurrentChecker()

and then later, during the human-player handling, you update it:

checker = input('Choose you checker').upper()
if deck.check_position(checker):
 if deck.check_checker_pos(checker):
 current_checker = checker_pos.coord(checker)

Problematically, you are calling methods on the deck class before you update the current_checker instance.

Suggestions

This class doesn't do anything. Either delete it and just put all the functionality in the Deck class, or make it an internal class of the Deck.

Since you have most of the functionality implemented in Deck already, I suggest just deleting this class and letting the deck handle everything.

Narrow View

In your main loop, you are doing a bunch of things:

• Tracking the current-player
• Invoking the player or bot turn code
• Implementing the move input mechanics
• Looping to validate input
• Updating the state of the Deck at each turn

To me, this says you need to work on the Deck class (see above), and also create another class or two. You need some code to handle player input mechanics and input validation logic. You need code to handle the simple gameplay mechanics.

I'd suggest creating a Player class, similar to the Bot class. Then you could just invoke the "play_turn()" method on two different objects.

The current-player problem can be solved by just calling players in sequence, as shown above.

The move mechanics and input validation are both part of the player interface. You could actually write different classes with different mechanics, and try them. Or make them play options, if you find that different people like different mechanics.

There should be no reason to update the deck state at the end of the turn. The deck should know enough about game play to update itself (it's just to track whose turn it is...).

deck_and_cheez.py

First, what's with the name? Why and_cheez?

class Deck

You have methods in this class that begin with __. Don't do this.

print_current_deck

Your numbers list should just be a string.

Your loop could use enumerate to eliminate the letter_count variable.

I'd suggest just hard-coding 10 print statements. It would be about the same length and would make the output more clear:

print(f"\t 1 2 3 4 5 6 7 8\n")
print(f"A\t{'|'.join(self.deck[0])}\tA")
print(f"B\t{'|'.join(self.deck[1])}\tB")
...

But really, it isn't the job of this class to communicate with the user. So instead of printing anything, just join the strings with a newline and return the resulting string:

return "\n".join(f"\t 1 2 3 4 5 6 7 8\n",
 f"A\t{'|'.join(self.deck[0])}\tA",
 ...)

__coordinates

Delete the '__'.

Rename this to express what it does: parse_user_coordinates

Use str.index to parse the input. It produces a more compact function:

row = "ABCDEFGH".index(usr_inp[0].upper())
col = "12345678".index(usr_inp[1])
return (row, col)

This will raise an exception if either input character is not matched. I think that's a good way to return control to the code that's driving the user interface, but you may want to catch the exception and do something different.

check_position

This is redundant with the method above. You print error messages, but it isn't the job of this class to communicate with the user. Better, IMO, to return a value or raise an exception.

calculate_possible_moves

I don't understand this method. You spend a fair amount of code computing two variables that are local to the method. Then you return, without storing, processing, or returning those variables.

The code is not dead -- there are two references to this method. But I think it isn't doing anything.

calculate_possible_move_for_check

This function is described as "calculate". But your return values are boolean. This is not a calculation at all.

You use the color of the checker to determine a direction of movement. This means that the code applies to "men" but not to queens. That in turn suggests to me that you probably want to make the individual pieces members of a class, instead of just character strings.

Finally, the board is fixed in size. You should pre-compute the results of this function and store them in static data. That reduces the function to a lookup.

attack

You can use unpacking to assign multiple variables:

u_x, u_y = *usr_inp

There are three possibilities for a square. You check two of them to make sure the third is true:

if self.deck[u_x][u_y] != ' ' and self.deck[u_x][u_y] != self.color:

Just test for what you want:

if self.deck[u_x][u_y] == self.color:

Checking for ' ' and for self.color at a target address suggests that your class needs more methods:

if self.deck[u_x - 1][u_y + 1] == ' ': # Up right

Could become:

target = self.up_right_of(usr_inp)
if self.is_empty(target):

With function self.is_enemy_of(color, location) available as well.

If you write methods for the various directions, you can iterate over the methods, which should shorten this code a lot. Instead of separate sections for x+1, y-1 and x-1,y-1 and ..., just make a tuple and iterate over it:

for direction in self.up_right_of, self.up_left_of, self.down_right_of, self.down_left_of:
 target = direction(usr_inp)

move

You behave in different ways based on some attribute of the data. That is a key indicator that you need a class. I suggest making two classes: "men" and "queens", and defining the __str__ method to return the current string values.

This code is too complex:

if self.color == '●くろまる' and cur_check in self.queen_list_w or self.color != '●くろまる' and cur_check in self.queen_list_b:

You check for (color is black) or (color is not black). That's always going to be true. Just delete the color discrimination, and check for membership in the queen lists.

But really, just make classes for the pieces and delete all this code.

bot.py

class Bot

There's a lot of redundant data stored in this class. You've got the deck, the checkers, enemy checkers, queens. All of which is also in the deck.

I suggest that you look hard at how you are using the data, and implement methods in the Deck class to provide that data instead. This would mean data being in just one place, eliminating a source of error.

I'm going to skip further review of this, since I think the suggestion to create objects for the pieces and move the data back into the Deck class will change this class pretty much everywhere.