Repository: munichpavel/clovek-ne-jezi-se

Issue:
Remove unused methods
Like `Game.assign_to_space`

Discussion:

Patch:
diff --git a/clovek_ne_jezi_se/game.py b/clovek_ne_jezi_se/game.py
index ad7cb84..8dc6221 100644
--- a/clovek_ne_jezi_se/game.py
+++ b/clovek_ne_jezi_se/game.py
@@ -128,7 +128,6 @@ class Move:
     kind = attr.ib()
     roll = attr.ib(kw_only=True, default=None)
     start = attr.ib(kw_only=True, default=None, validator=check_start)
-#    end = attr.ib(kw_only=True, default=None)
 
     @kind.validator
     def check_kind(self, attribute, value):
@@ -149,7 +148,7 @@ class Game:
         self.n_players = len(self.players)
         self._validate_n_players()
         self._set_player_symbols()
-        self._set_player_start()
+        self._set_player_leave_waiting()
         self._set_player_prehome()
 
     def _validate_n_players(self):
@@ -177,7 +176,7 @@ class Game:
 
         self.player_symbols = symbols
 
-    def _set_player_start(self):
+    def _set_player_leave_waiting(self):
         for idx, player in enumerate(self.players):
             player.set_leave_waiting_position(idx, self.section_length)
 
@@ -226,13 +225,15 @@ class Game:
 
         return self.get_waiting_count_array()[private_symbol]
 
-    def set_waiting_count_array(self, symbol, count):
+    def set_symbol_waiting_count(self, symbol, count):
+        """Set symbol waiting count value"""
         self.board.waiting_count[symbol] = count
 
         private_symbol = self.board.get_private_symbol(symbol)
         self._waiting_count[private_symbol] = count
 
     def initialize_spaces_array(self):
+        """Initialize internal spaces representation"""
         res = [
             self.board.get_private_symbol(symbol)
             for symbol in self.board.spaces
@@ -242,13 +243,14 @@ class Game:
     def get_spaces_array(self):
         return self._spaces_array
 
-    def set_space_array(self, symbol, position):
+    def set_symbol_space_array(self, symbol, position):
         self.board.spaces[position] = symbol
 
         private_symbol = self.board.get_private_symbol(symbol)
         self._spaces_array[position] = private_symbol
 
     def initialize_homes_array(self):
+        """Initialize internal homes representation"""
         res = []
         for symbol in self.player_symbols:
             res.append([
@@ -276,17 +278,8 @@ class Game:
 
         return self.get_homes_array()[private_symbol, :]
 
-    def set_homes_array(self, symbol, position):
-        self.board.homes[symbol][position] = symbol
-
-        private_symbol = self.board.get_private_symbol(symbol)
-        self._homes_array[private_symbol, position] = private_symbol
-
-    def assign_to_space(self, symbol, idx):
-        """Convenience function. TODO: Deprecate or make private?"""
-        self._spaces_array[idx] = self.board.get_private_symbol(symbol)
-
     def roll(self):
+        """Roll dice method"""
         res = self._get_roll_value()
         if self.roll_is_valid(res):
             return res
@@ -350,7 +343,7 @@ class Game:
 
     def move_factory(self, symbol, kind, roll, start=None):
         """
-        Create valid Move instances corresponding to input
+        Create valid Move instances corresponding to input.
 
         Parameters
         ----------
@@ -563,6 +556,15 @@ class Game:
 
     # Space to home move methods
     def space_to_home_validator(self, move):
+        """
+        Parameters
+        ----------
+        move : Move
+
+        Returns
+        -------
+        res : Boolean
+        """
         end = self.get_space_to_home_position(
             move.symbol, move.roll, move.start
         )
@@ -609,6 +611,15 @@ class Game:
 
     # Home advance move methods
     def home_advance_validator(self, move):
+        """
+        Parameters
+        ----------
+        move : Move
+
+        Returns
+        -------
+        res : Boolean
+        """
         return self._home_advance_is_valid(move.symbol, move.roll, move.start)
 
     def _home_advance_is_valid(self, symbol, roll, start):
@@ -656,7 +667,16 @@ class Game:
     def return_to_waiting_validator(self, move):
         """
         Returns true, as return_to_waiting is generated within this class,
-        as it is an effect of another agent move
+        as it is an effect of another agent move.
+
+        Parameters
+        ----------
+        move : Move
+
+        Returns
+        -------
+        res : Boolean
+            Always returns True
         """
         return True
 
@@ -687,27 +707,33 @@ class Game:
 
     def _leave_waiting_updater(self, move):
         current_count = self.get_symbol_waiting_count(move.symbol)
-        self.set_waiting_count_array(move.symbol, current_count - 1)
+        self.set_symbol_waiting_count(move.symbol, current_count - 1)
         start = self.get_player(move.symbol).get_leave_waiting_position()
-        self.set_space_array(move.symbol, start)
+        self.set_symbol_space_array(move.symbol, start)
 
     def _space_advance_updater(self, move):
-        self.set_space_array(EMPTY_SYMBOL, move.start)
-        self.set_space_array(move.symbol, move.start + move.roll)
+        self.set_symbol_space_array(EMPTY_SYMBOL, move.start)
+        self.set_symbol_space_array(move.symbol, move.start + move.roll)
 
     def _space_to_home_updater(self, move):
-        self.set_space_array(EMPTY_SYMBOL, move.start)
+        self.set_symbol_space_array(EMPTY_SYMBOL, move.start)
         end = self.get_space_to_home_position(
             move.symbol, move.roll, move.start
         )
         self.set_homes_array(move.symbol, end)
 
+    def set_homes_array(self, symbol, position):
+        self.board.homes[symbol][position] = symbol
+
+        private_symbol = self.board.get_private_symbol(symbol)
+        self._homes_array[private_symbol, position] = private_symbol
+
     def _return_to_waiting_updater(self, move):
         """
         Only update waiting counts, as the move causing return to waiting
         updates the spaces
         """
         pre_return_waiting_counts = self.get_symbol_waiting_count(move.symbol)
-        self.set_waiting_count_array(
+        self.set_symbol_waiting_count(
             move.symbol, pre_return_waiting_counts + 1
         )


Test patch:
diff --git a/tests/test_clovek.py b/tests/test_clovek.py
index 329551d..d0547df 100644
--- a/tests/test_clovek.py
+++ b/tests/test_clovek.py
@@ -202,23 +202,6 @@ class TestGameSetup:
         assert isinstance(game_array, np.ndarray)
         np.testing.assert_array_equal(game_array, expected)
 
-    @pytest.mark.parametrize(
-        'symbol_idx,space_idx',
-        [(0, 0), (1, 0), (3, 1)]
-    )
-    def test_assignments(self, symbol_idx, space_idx):
-        symbol = self.mini_game.player_symbols[symbol_idx]
-        self.mini_game.assign_to_space(symbol, space_idx)
-
-        # Define expected array by modifying spaces array
-        expected = self.mini_game.get_spaces_array()
-        expected[space_idx] = symbol_idx
-
-        np.testing.assert_array_equal(
-            self.mini_game.get_spaces_array(),
-            expected
-        )
-
     def test_dice_roll_monkeypatch(self, monkeypatch):
         monkeypatch.setattr(self.mini_game, 'roll', lambda: monkey_roll(1))
         assert self.mini_game.roll_is_valid(self.mini_game.roll())
@@ -266,13 +249,13 @@ class TestGameMoves:
         assert self.game.get_symbol_waiting_count('1') == PIECES_PER_PLAYER
 
         modified_game = deepcopy(self.game)
-        modified_game.set_waiting_count_array('1', 0)
+        modified_game.set_symbol_waiting_count('1', 0)
         assert modified_game.get_symbol_waiting_count('1') == 0
 
     def test_get_symbol_space_position_array(self):
         modified_game = deepcopy(self.game)
-        modified_game.set_space_array('1', 0)
-        modified_game.set_space_array('2', 1)
+        modified_game.set_symbol_space_array('1', 0)
+        modified_game.set_symbol_space_array('2', 1)
 
         assert modified_game.get_symbol_space_array('1') == np.array([0])
         assert modified_game.get_symbol_space_array('2') == np.array([1])
@@ -307,7 +290,7 @@ class TestGameMoves:
     @pytest.mark.parametrize('position,symbol', [(0, '1'), (1, EMPTY_SYMBOL)])
     def test_get_space_occupier(self, position, symbol):
         modified_game = deepcopy(self.game)
-        modified_game.set_space_array('1', 0)
+        modified_game.set_symbol_space_array('1', 0)
 
         assert modified_game.get_space_occupier(position) == symbol
 
@@ -385,13 +368,13 @@ class TestGameMoves:
     def test_move_factory(self, arg_dict, expected):
         modified_game = deepcopy(self.game)
 
-        modified_game.set_space_array('1', 1)
-        modified_game.set_space_array('1', self.prehome_positions['1'])
-        modified_game.set_waiting_count_array('1', PIECES_PER_PLAYER - 2)
+        modified_game.set_symbol_space_array('1', 1)
+        modified_game.set_symbol_space_array('1', self.prehome_positions['1'])
+        modified_game.set_symbol_waiting_count('1', PIECES_PER_PLAYER - 2)
 
-        modified_game.set_space_array('2', self.leave_waiting_positions['1'])
-        modified_game.set_space_array('2', self.prehome_positions['1'] - 1)
-        modified_game.set_waiting_count_array('2', PIECES_PER_PLAYER - 1)
+        modified_game.set_symbol_space_array('2', self.leave_waiting_positions['1'])
+        modified_game.set_symbol_space_array('2', self.prehome_positions['1'] - 1)
+        modified_game.set_symbol_waiting_count('2', PIECES_PER_PLAYER - 1)
 
         move = modified_game.move_factory(**arg_dict)
 
@@ -420,9 +403,9 @@ class TestGameMoves:
     )
     def test_move_factory_errors(self, symbol, kind, roll, start):
         modified_game = deepcopy(self.game)
-        modified_game.set_waiting_count_array('1', 0)
+        modified_game.set_symbol_waiting_count('1', 0)
 
-        modified_game.set_space_array('2', self.leave_waiting_positions['2'])
+        modified_game.set_symbol_space_array('2', self.leave_waiting_positions['2'])
 
         modified_game.set_homes_array('3', 0)
         modified_game.set_homes_array('4', 2)
@@ -463,11 +446,11 @@ class TestGameMoves:
     )
     def test_get_moves_of(self, symbol, kind, roll, expected):
         modified_game = deepcopy(self.game)
-        modified_game.set_waiting_count_array('2', 0)
-        modified_game.set_space_array(
+        modified_game.set_symbol_waiting_count('2', 0)
+        modified_game.set_symbol_space_array(
              '2', self.leave_waiting_positions['2']
         )
-        modified_game.set_space_array(
+        modified_game.set_symbol_space_array(
              '3', self.prehome_positions['3']
         )
         modified_game.set_homes_array('4', 1)
@@ -482,15 +465,15 @@ class TestGameMoves:
         assert game_state_is_equal(self.game, self.game)
 
         modified_waiting = deepcopy(self.game)
-        modified_waiting.set_waiting_count_array('1', 0)
+        modified_waiting.set_symbol_waiting_count('1', 0)
         assert not game_state_is_equal(self.game, modified_waiting)
 
         modified_spaces = deepcopy(self.game)
-        modified_spaces.set_space_array('1', 0)
+        modified_spaces.set_symbol_space_array('1', 0)
         assert not game_state_is_equal(self.game, modified_spaces)
 
         modified_homes = deepcopy(self.game)
-        modified_homes.set_space_array('1', 0)
+        modified_homes.set_symbol_space_array('1', 0)
         assert not game_state_is_equal(self.game, modified_homes)
 
     def test_do(self):
@@ -499,18 +482,18 @@ class TestGameMoves:
 
         # Prepopulate board
         for game in [modified_game, expected_game]:
-            game.set_space_array('2', self.leave_waiting_positions['1'])
-            game.set_waiting_count_array('2', PIECES_PER_PLAYER - 1)
+            game.set_symbol_space_array('2', self.leave_waiting_positions['1'])
+            game.set_symbol_waiting_count('2', PIECES_PER_PLAYER - 1)
 
-            game.set_space_array('4', self.prehome_positions['4'])
-            game.set_waiting_count_array('4', PIECES_PER_PLAYER - 1)
+            game.set_symbol_space_array('4', self.prehome_positions['4'])
+            game.set_symbol_waiting_count('4', PIECES_PER_PLAYER - 1)
 
         # Leave waiting and send opponent piece back to waiting
         modified_game.do('1', 'leave_waiting', roll=NR_OF_DICE_FACES)
-        expected_game.set_waiting_count_array('1', PIECES_PER_PLAYER - 1)
+        expected_game.set_symbol_waiting_count('1', PIECES_PER_PLAYER - 1)
         pre_move_waiting_count = expected_game.get_symbol_waiting_count('2')
-        expected_game.set_waiting_count_array('2',  pre_move_waiting_count + 1)
-        expected_game.set_space_array('1', self.leave_waiting_positions['1'])
+        expected_game.set_symbol_waiting_count('2',  pre_move_waiting_count + 1)
+        expected_game.set_symbol_space_array('1', self.leave_waiting_positions['1'])
 
         assert game_state_is_equal(modified_game, expected_game)
 
@@ -522,7 +505,7 @@ class TestGameMoves:
         modified_game.do(
             '4', 'space_to_home', roll=1, start=self.prehome_positions['4']
         )
-        expected_game.set_space_array(
+        expected_game.set_symbol_space_array(
             EMPTY_SYMBOL, self.prehome_positions['4']
         )
         expected_game.set_homes_array('4', 0)
@@ -535,7 +518,7 @@ class TestGameMoves:
         occupied_position = (
             modified_game.get_player('3').get_leave_waiting_position()
         )
-        modified_game.set_space_array('1', occupied_position)
+        modified_game.set_symbol_space_array('1', occupied_position)
 
         assert modified_game.board.spaces[occupied_position] == '1'