Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Solution' #527

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

'Solution' #527

wants to merge 1 commit into from

Conversation

NazarSlavych
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Great job on implementing the Battleship game! 🎉 Your code meets the task requirements and constraints, and the core functionality is well-executed. Here are some key points and suggestions for improvement:

  1. Functionality: The Battleship class and its methods are implemented as required. The fire method correctly returns "Miss!", "Hit!", or "Sunk!" based on the state of the targeted cell.

  2. Visualization: The print_field method is implemented to visualize the field. However, it currently assumes a fixed field size of 10x10. If you plan to support different field sizes, consider making this configurable.

  3. Validation: The _validate_field method is defined to check the conditions for the number and types of ships. However, it is not called anywhere, which means the validation logic is not utilized. Consider invoking this method during the initialization of the Battleship class to ensure the field is valid.

  4. Code Style: The code adheres to the guidelines, avoiding redundant statements and using attributes effectively.

  5. Critical Issue: The _validate_field method does not currently check that ships are not located in neighboring cells, even diagonally. This is a requirement that should be addressed to fully meet the task specifications.

Your code is approved to help you move forward with your learning journey. Please review the comments and suggestions to improve your code. Addressing these issues will enhance the functionality and robustness of your implementation. Keep up the great work and continue learning from your experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

Comment on lines +83 to +94
def _validate_field(self) -> bool:
ship_counts = {1: 0, 2: 0, 3: 0, 4: 0}
for ship in self.ships:
length = len(ship.decks)
if length in ship_counts:
ship_counts[length] += 1
return (
ship_counts[1] == 4
and ship_counts[2] == 3
and ship_counts[3] == 2
and ship_counts[4] == 1
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _validate_field method is defined but never called. Consider calling this method during the initialization of the Battleship class to ensure that the field is valid according to the specified ship counts.

return "Hit!"

def print_field(self) -> None:
field_visual = [["~" for _ in range(10)] for _ in range(10)]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The print_field method assumes a fixed field size of 10x10. If the game is intended to support different field sizes, consider making the field size configurable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants