Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Great work on this task! Your solution correctly implements the core requirements, and the class structure for Battleship, Ship, and Deck is well-organized. The fire method behaves exactly as expected.
I am approving your submission. For future improvements, here are a couple of minor points to consider:
- In
app/main.py, there is a typo in the_validade_coordmethod name. It should be_validate_coordfor correctness. - The
is_drownedparameter in theShipclass constructor is redundant. A ship always starts with all decks intact, so you can remove this parameter and setself.is_drowned = Falsedirectly within the__init__method.
These are just suggestions for refinement and don't affect the functionality. Keep up the excellent work!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
| def __init__(self, | ||
| start: Coord, | ||
| end: Coord, | ||
| is_drowned: bool = False |
There was a problem hiding this comment.
The is_drowned parameter is unnecessary here. A ship should never be in a 'drowned' state when it's first created, as all its decks are initialized as 'alive'. It's better to set self.is_drowned = False directly within the __init__ method to prevent creating objects with an inconsistent state.
| raise ValueError("Overlapping ships detected.") | ||
| self.field[deck.coord] = ship | ||
|
|
||
| def _validade_coord(self, coord: Coord) -> None: |
There was a problem hiding this comment.
There's a typo in the method name. It should be _validate_coord. Remember to correct the calls to this method on lines 82 and 100 as well.
2 participants
No description provided.