Repository: munichpavel/clovek-ne-jezi-se

Issue:
Move dice roll from agent / player to game
Not sure best place to have dice roll, but there is no reason to have it along with agent definitions.

Discussion:

Patch:
diff --git a/clovek_ne_jezi_se/agent.py b/clovek_ne_jezi_se/agent.py
index 715db04..49cd9ea 100644
--- a/clovek_ne_jezi_se/agent.py
+++ b/clovek_ne_jezi_se/agent.py
@@ -1,7 +1,7 @@
-from random import randint
+
 import attr
 
-from .consts import EMPTY_SYMBOL, NR_OF_DICE_FACES
+from .consts import EMPTY_SYMBOL
 
 
 @attr.s
@@ -37,22 +37,6 @@ class Player:
     def get_prehome_position(self):
         return self._prehome_position
 
-    def roll(self):
-        res = Player._get_roll_value()
-        if Player.roll_is_valid(res):
-            print("Player {} rolls a {}".format(self.symbol, res))
-            return res
-        else:
-            raise ValueError('Roll value must be between 1 and 6')
-
-    @staticmethod
-    def _get_roll_value():
-        return randint(1, NR_OF_DICE_FACES)
-
-    @staticmethod
-    def roll_is_valid(roll_value):
-        return 1 <= roll_value <= NR_OF_DICE_FACES
-
 
 class FurthestAlongAgent(Player):
     """Agent who always moves the game piece furthest along"""
diff --git a/clovek_ne_jezi_se/game.py b/clovek_ne_jezi_se/game.py
index 25b3a4e..ad7cb84 100644
--- a/clovek_ne_jezi_se/game.py
+++ b/clovek_ne_jezi_se/game.py
@@ -1,5 +1,6 @@
 """Clovek ne jezi se game board and plays"""
 from math import floor
+from random import randint
 
 import attr
 
@@ -232,7 +233,10 @@ class Game:
         self._waiting_count[private_symbol] = count
 
     def initialize_spaces_array(self):
-        res = [self.board.get_private_symbol(symbol) for symbol in self.board.spaces]
+        res = [
+            self.board.get_private_symbol(symbol)
+            for symbol in self.board.spaces
+        ]
         self._spaces_array = np.array(res)
 
     def get_spaces_array(self):
@@ -282,6 +286,20 @@ class Game:
         """Convenience function. TODO: Deprecate or make private?"""
         self._spaces_array[idx] = self.board.get_private_symbol(symbol)
 
+    def roll(self):
+        res = self._get_roll_value()
+        if self.roll_is_valid(res):
+            return res
+        else:
+            raise ValueError(f'Invalid roll value: {res}')
+
+    def _get_roll_value(self):
+        return randint(1, NR_OF_DICE_FACES)
+
+    def roll_is_valid(self, value):
+        return 1 <= value and value <= NR_OF_DICE_FACES
+
+    # Game moves
     def get_moves_of(self, symbol, kind, roll):
         res = []
         starts = self.get_move_starts(symbol, kind)


Test patch:
diff --git a/tests/test_clovek.py b/tests/test_clovek.py
index efd5754..329551d 100644
--- a/tests/test_clovek.py
+++ b/tests/test_clovek.py
@@ -25,14 +25,6 @@ class TestPlayer:
     with pytest.raises(TypeError):
         Player(1, number_of_players=4)
 
-    def test_dice_roll_monkeypatch(self, monkeypatch):
-
-        monkeypatch.setattr(self.player, 'roll', lambda: monkey_roll(1))
-        assert self.player.roll_is_valid(self.player.roll())
-
-        monkeypatch.setattr(self.player, 'roll', lambda: monkey_roll(0))
-        assert ~self.player.roll_is_valid(self.player.roll())
-
 
 @pytest.fixture
 def small_initial_board():
@@ -73,7 +65,7 @@ class TestBoard:
              ) == symbol
 
 
-class TestGame:
+class TestGameSetup:
     players = []
     for symbol in ['1', '2', '3', '4']:
         player = Player(symbol=symbol, number_of_players=4)
@@ -155,7 +147,7 @@ class TestGame:
             ('4', 30)
         ]
     )
-    def test_player_normal_start(
+    def test_player_normal_leave_waiting(
         self, symbol, expected_position
     ):
         # Normal board
@@ -227,6 +219,13 @@ class TestGame:
             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())
+
+        monkeypatch.setattr(self.mini_game, 'roll', lambda: monkey_roll(0))
+        assert ~self.mini_game.roll_is_valid(self.mini_game.roll())
+
 
 class TestMoves:
     def test_validators(self):
@@ -243,7 +242,7 @@ class TestMoves:
             Move('1', 'leave_waiting', roll=NR_OF_DICE_FACES, start=0)
 
 
-class TestGameAction:
+class TestGameMoves:
     symbols = ['1', '2', '3', '4']
     players = []
     for symbol in symbols: