Fixing the mangled formatting, as best as I am able (and can be
bothered).
On Thu, Jan 01, 2015 at 11:48:18PM -0500, WolfRage wrote:
class GameTile():
def __init__(self, id, **kwargs):
# id is (X,Y)
self.id = id
What is the purpose of the **kwargs? It doesn't get used, it just
silently ignores them.
Actually, what is the purpose of this GameTile class? It has no methods
apart from the constructor. That is a sign of a class that isn't pulling
its weight, its not doing anything. All the other code in your program
looks inside the GameTile and accesses self.id directly. This is a sure
sign of a class that doesn't need to exist.
It may be better to give this at least a __str__ method, so if
nothing else you can print it without looking inside:
class GameTile():
def __init__(self, x, y):
self.x = x
self.y = y
def __str__(self):
return "%d, %d" % (self.x, self.y)
class GameGrid():
def __init__(self, **kwargs):
self.cols = 7
self.rows = 8
# grid is 2d array as y, x ie [y][x].
self.grid = [[None] * self.rows for i in range(self.cols)]
Why do you have GameTiles use X,Y but the grid uses the opposite order,
Y,X? This is going to cause confusion. I'm already confused!
def make_grid(self):
for row in range(0, self.rows):
for col in range(0, self.cols):
self.grid[col][row] = GameTile(id=str(row) + ',' + str(col))
No need to write range(0, Whatever), since range defaults to 0. Better
to write range(Whatever).
Why does the GameTile record the coordinates as strings?
I would prefer something like this:
def make_grid(self):
for row in range(self.rows):
for col in range(self.cols):
self.grid[col][row] = GameTile(row, col)
although it looks strange due to the backwards ordering of col/row.
Your __init__ method should automatically call make_grid. That is:
def __init__(self, **kwargs):
self.cols = 7
self.rows = 8
# grid is 2d array as y, x ie [y][x].
self.grid = [[None] * self.rows for i in range(self.cols)]
self.make_grid()
Actually, even better will be to move the self.grid = ... line out of
the __init__ method, and make it part of make_grid. Since it is part of
making the grid.
def print_by_row(self):
for col in range(0, self.cols):
for row in range(0, self.rows):
print(self.grid[col][row].id)
This now becomes:
def print_by_row(self):
for col in range(0, self.cols):
for row in range(0, self.rows):
print(self.grid[col][row])
since GameTiles now know how to print themselves. Likewise for this:
def print_by_col(self):
for row in range(0, self.rows):
for col in range(0, self.cols):
print(self.grid[col][row])
Another simplification here:
def check_bounds(self, x, y):
return (0 <= x < self.rows) and (0 <= y < self.cols)
Your lookup_node method returns a GameTile or False on failure:
def lookup_node(self, x, y, ):
if not self.check_bounds(x, y):
return False
return self.grid[y][x]
I'm not sure about that design. I wonder whether it would be better to
return None, or raise an exception.
You have a lookup_node method that checks the bounds, but you don't seem
to use it anywhere. Why does it exist?
def draw_grid(self):
for col in range(0, self.cols):
print(end='| ')
for row in range(0, self.rows):
print(self.grid[col][row].id, end=' | ')
print()
I'm not sure if I have reconstructed the indentation correctly here. A
few changes: no need to call range(0, ...). GameTile instances now know
how to print themselves, so you can call:
print(self.grid[col][row], end=' | ')
This could be improved:
temp = GameGrid()
temp.make_grid()
temp.draw_grid()
Creating a GameGrid should automatically create the game grid, hence
what I said above about having __init__ call make_grid. Then use a more
meaningful name, and we have:
grid = GameGrid()
grid.draw_grid() # Maybe this could be just called "draw"?
--
Steven
_______________________________________________
Tutor maillist - Tutor@python.org
To unsubscribe or change subscription options:
https://mail.python.org/mailman/listinfo/tutor
