refactoring of battleship_oo2 #1
Conversation
added comments some variables are kept private via _ convention added Cell class
added comments some variables are kept private via _ convention added Cell class
added setters and getters
battleship_oo2_test.py
Outdated
| self.shooter = self._player1 | ||
| self.receiver = self._player2 | ||
| self._shooter = self._player1 | ||
| self._receiver = self._player2 | ||
|
|
||
| def play_game(self): |
There was a problem hiding this comment.
This is the only method I can see you use from outside Game which means all other Game methods should be private.
I think there are other examples for the same ie. public methods that should be private.
Consider creating all methods as private by default and only make those methods public that you especially want to expose to the outside world.
battleship_oo2_test.py
Outdated
| def __init__(self, coordinates: 'Coordinates', status: str = None): | ||
| self._coordinates = coordinates | ||
| self._status = status | ||
| self._allowed_statuses = {'Hit', 'Not Hit'} |
There was a problem hiding this comment.
Consider using an enum for constants like 'Hit' and 'Not Hit' to eliminate hardcoding these strings multiple times in the code. With an enum you can define valid statuses and use those same enum values across the board.
battleship_oo2_test.py
Outdated
| return False | ||
|
|
||
|
|
||
| class Board: | ||
| """This class keeps track of the shots taken by a Player """ | ||
| def __init__(self, board_len): | ||
|
|
||
| def __init__(self, board_len: int): | ||
| self.coordinates = [Coordinates(x=x, y=y) for x in range(1, board_len) for y in range(1, board_len)] |
There was a problem hiding this comment.
You populate coordinates but don't use it anywhere instead you use the shots_taken set.
Although this works with the current features, you should remove any unused code and I suggest you remove the shots_taken set and keep coordinates storing Cells instead of Coordinates.
If the Board consists of Cells you can register a shot on the board by setting the status of the cell to something like 'Undiscovered' by default and make it 'Empty' or 'Hit' after a shot and it's also easy to verify if a shot is valid on the board or not.
Board now contains Cells instead of Coordinates
…gregate asserts in one function
added comments
some variables are kept private via _ convention
added Cell class