patternpythonMinor
Snake is dead. Long live snake
Viewed 0 times
deadlivelongsnake
Problem
This is an iterative review.
Previous Iteration
My snake now moves and grows and successfully kills itself.
Eventually, the game board will handle state and things like the list of fatal coordinates and telling a snake whether it's dead, but for now that's just a function called
Fatal coordinates are
So, how pythonic is my snake?
And how can I make it better?
Module
(N.B. there are other things in the utiltiies module, but they're not used here so I left them out)
Module
(code laid out in the order presented here, but split up for convenience)
Header
Static Methods
```
def get_initial_body_coordinates(x_pos: int, y_pos: int, facing: str, length: int) -> list:
first_coordinate = np.array([x_pos, y_pos])
unit_vector = unit_vector_from_cardinal(facing)
return [
first_coordinate
- unit_vector * i
for i in range(0, length)
]
def is_fatal_position(position, *args):
# *args should be (
Previous Iteration
My snake now moves and grows and successfully kills itself.
Eventually, the game board will handle state and things like the list of fatal coordinates and telling a snake whether it's dead, but for now that's just a function called
fatal_board_positions paired with is_fatal_position.Fatal coordinates are
(X = 1-10, Y = 7-10)So, how pythonic is my snake?
And how can I make it better?
Module
utilities.py(N.B. there are other things in the utiltiies module, but they're not used here so I left them out)
import numpy as np
DIRECTION_VECTORS = {
'n': (0, 1),
's': (0, -1),
'e': (1, 0),
'w': (-1, 0)
}
def unit_vector_from_cardinal(cardinal: str) -> np.array:
if cardinal in DIRECTION_VECTORS:
return np.array(DIRECTION_VECTORS[cardinal])
else:
raise ValueError("An invalid cardinal direction was provided")Module
Snake.py(code laid out in the order presented here, but split up for convenience)
Header
import numpy as np
from utilities import unit_vector_from_cardinal
'''
Game Board:
X by Y array of numbers
Derived from Snake Object and base parameters (height, width, obstacles?)
Board Values:
0 - Empty
1 - Snake Head
2 - Snake Body
Snake:
Ordered list of X,Y coordinates for each body segment
Actions:
The board is responsible for handling state
The snake is therefore ignorant of state and should not decide *how* to move
The board should decide whether the snake:
Grows (food)
Moves (non-food)
Is Dead
'''Static Methods
```
def get_initial_body_coordinates(x_pos: int, y_pos: int, facing: str, length: int) -> list:
first_coordinate = np.array([x_pos, y_pos])
unit_vector = unit_vector_from_cardinal(facing)
return [
first_coordinate
- unit_vector * i
for i in range(0, length)
]
def is_fatal_position(position, *args):
# *args should be (
Solution
Glad to see iteration #2 of your code, and that you've used some of my suggestions! :)
Got some more for you though.
Docstrings
While you have comments explaining what different functions seem to do (or they are already fairly clear to someone who understands what's going on with the code), you should consider using docstrings in your code.
Style: PEP8 Guidelines
Just some minor things here.
Snake class: Multiple suggestions
Make
This is more or less to protect against modification of the Snake positions and to use a Getter that returns the list of positions, but don't directly access the protected variable inside the class. (We have other 'getter' and 'setter' items too, so I'll touch base on that in a few minutes)
Note that I also add a property called
'Getter' functions as properties
These are getters:
The getter functions are simply returning information. We can handle them as properties instead of standard functions (thereby it's not 'callable', so we save some parentheses later on, and refer to them without parentheses later). We can also change their name by setting them as properties.
Anywhere you had
You don't need to create a new variable -
There may be more suggestions to come, though this is the first set of my recommendations.
Got some more for you though.
Docstrings
While you have comments explaining what different functions seem to do (or they are already fairly clear to someone who understands what's going on with the code), you should consider using docstrings in your code.
Style: PEP8 Guidelines
Just some minor things here.
- There should not be any spaces between
printand the(that goes with it.
- In line comments at the end of a line should have two spaces between the end of the code line, and the start of the comment (the
#)
Snake class: Multiple suggestions
Make
body_positions list a protected item in the class, add a Getter that works in this case.This is more or less to protect against modification of the Snake positions and to use a Getter that returns the list of positions, but don't directly access the protected variable inside the class. (We have other 'getter' and 'setter' items too, so I'll touch base on that in a few minutes)
class Snake:
...
_body_coordinates = list()
...
@property
def body_coordinates(self) -> list:
return self._body_coordinates
...Note that I also add a property called
body_coordinates which basically does what you were doing before. (Python won't yell for accessing the protected property, necessarily, but we should use Getters to get data from the class's items unless we are directly manipulating that class's properties with a function or code outside of the class, which doesn't seem to be being done here. Accessing protected properties can cause undefined behavior at times, so it's better to be careful here and use Getters / Setters / Modifiers instead.)'Getter' functions as properties
These are getters:
def get_non_head_coordinates(self) -> list:
return self.body_coordinates[1:]
def get_head_pos(self):
return self.body_coordinates[0]The getter functions are simply returning information. We can handle them as properties instead of standard functions (thereby it's not 'callable', so we save some parentheses later on, and refer to them without parentheses later). We can also change their name by setting them as properties.
class Snake:
...
@property
def non_head_coordinates(self) -> list:
return self._body_coordinates[1:]
@property
def head_pos(self):
return self._body_coordinates[0]
...Anywhere you had
.get_* for getting info from the Snake class, you can replace it with just .head_pos and .non_head_coordinates. No need for parentheses since you're not really needing to worry about calling anything here as we have nothing more to do but getting the value from an internal item to the Snake class.grow: Unnecessary variable creation inside classYou don't need to create a new variable -
current_head_pos will just use a property of the class with the above recommendations, and since the variable is only being used inside of the grow function, we don't need to have a variable here. Just use the property in place, so you'll end up with this:def grow(self, direction: str):
# Add a new coordinate to the head of the body list
direction_vector = unit_vector_from_cardinal(direction)
new_pos = self.head_pos + direction_vector
self.add_new_pos(new_pos)There may be more suggestions to come, though this is the first set of my recommendations.
Code Snippets
class Snake:
...
_body_coordinates = list()
...
@property
def body_coordinates(self) -> list:
return self._body_coordinates
...def get_non_head_coordinates(self) -> list:
return self.body_coordinates[1:]
def get_head_pos(self):
return self.body_coordinates[0]class Snake:
...
@property
def non_head_coordinates(self) -> list:
return self._body_coordinates[1:]
@property
def head_pos(self):
return self._body_coordinates[0]
...def grow(self, direction: str):
# Add a new coordinate to the head of the body list
direction_vector = unit_vector_from_cardinal(direction)
new_pos = self.head_pos + direction_vector
self.add_new_pos(new_pos)Context
StackExchange Code Review Q#143784, answer score: 4
Revisions (0)
No revisions yet.