[Tutor] Making Doubly Linked List with Less Lines of Code.

[Steven D'Aprano]

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