first python: maze with backtracker

Question

i'm learning python but i'm used to java development. this might have influenced my programming style.

feel free to give any feedback on the code

maze_learning.py:

import maze
def main():
 my_maze = maze.Maze(64, 64)
 my_maze.back_tracker()
 my_maze.print_maze()
if __name__ == "__main__":
 main()

maze.py:

import cell
import random
def pop(stack):
 item = stack[len(stack) - 1]
 del stack[len(stack) - 1]
 return item
def push(item, stack):
 size = len(stack)
 stack[size] = item
def add_if_unvisited(maze, pos, target):
 if pos in maze:
 current = maze[pos]
 if current.visited == 0:
 target.append(current)
def carve(from_cell, to_cell):
 if to_cell.x_pos - from_cell.x_pos == 1:
 from_cell.door_east = 1
 to_cell.door_west = 1
 if to_cell.x_pos - from_cell.x_pos == -1:
 from_cell.door_west = 1
 to_cell.door_east = 1
 if to_cell.y_pos - from_cell.y_pos == -1:
 from_cell.door_north = 1
 to_cell.door_south = 1
 if to_cell.y_pos - from_cell.y_pos == 1:
 from_cell.door_south = 1
 to_cell.door_north = 1
class Maze:
 width = 0
 height = 0
 cells = {}
 def __init__(self, width, height):
 self.width = width
 self.height = height
 for dy in range(0, height):
 for dx in range(0, width):
 self.cells[complex(dx, dy)] = cell.Cell(dx, dy)
 # see https://en.wikipedia.org/wiki/Maze_generation_algorithm
 def back_tracker(self):
 stack = {}
 current = self.cells[complex(random.randint(0, self.width - 1), random.randint(0, self.height - 1))]
 current.visited = 1
 push(current, stack)
 while len(stack) > 0:
 current = pop(stack)
 unvisited = self.list_unvisited(current)
 if len(unvisited) > 0:
 push(current, stack)
 random_index = random.randint(0, len(unvisited) - 1)
 chosen = unvisited[random_index]
 carve(chosen, current)
 current = chosen
 current.visited = 1
 push(current, stack)
 def list_unvisited(self, current):
 pos_north = complex(current.x_pos, current.y_pos - 1)
 pos_east = complex(current.x_pos + 1, current.y_pos)
 pos_south = complex(current.x_pos, current.y_pos + 1)
 pos_west = complex(current.x_pos - 1, current.y_pos)
 neighbors = []
 add_if_unvisited(self.cells, pos_north, neighbors)
 add_if_unvisited(self.cells, pos_east, neighbors)
 add_if_unvisited(self.cells, pos_south, neighbors)
 add_if_unvisited(self.cells, pos_west, neighbors)
 return neighbors
 def print_maze(self):
 for dy in range(0, self.height):
 top_row = ""
 mid_row = ""
 bot_row = ""
 for dx in range(0, self.width):
 top_row = top_row + self.cells[complex(dx, dy)].top_row()
 mid_row = mid_row + self.cells[complex(dx, dy)].mid_row()
 bot_row = bot_row + self.cells[complex(dx, dy)].bot_row()
 print(top_row)
 print(mid_row)
 print(bot_row)

cell.py:

class Cell:
 door_north = 0
 door_east = 0
 door_west = 0
 door_south = 0
 visited = 0
 x_pos = 0
 y_pos = 0
 def __init__(self, xp, yp):
 self.x_pos = xp
 self.y_pos = yp
 def top_row(self):
 door = "###"
 if self.door_north == 1:
 door = " "
 return "#" + door + "#"
 def mid_row(self):
 left_door = "#"
 if self.door_west == 1:
 left_door = " "
 right_door = "#"
 if self.door_east == 1:
 right_door = " "
 return left_door + " " + right_door
 def bot_row(self):
 door = "###"
 if self.door_south == 1:
 door = " "
 return "#" + door + "#"

this is, as said, my first code with python

Answer 1

First some truth in advertising: I did not fully investigate the code and assess everything. Basically I ran it and made various edits, ensuring that it still worked after each of my changes. I sort of jumped around and stopped when I ran out of energy. Perhaps someone else will consider the other aspects of the program and provide more thorough feedback. For a first Python program, you're off to a very good start.

Declare attributes at the appropriate level. Some Python programmers have a habit of initializing instance-level attributes at the class level, rather than in __init__(). They defend the practice under the it-works theory. Indeed it does work, due to Python's lookup behavior: when it fails to find an attribute on self, it turns to the class of self. However, from a code readability perspective, it's an ill-advised shortcut. Your code is already quite readable, so you should join Team Readability and declare instance-level attributes inside of __init__().

Just use a tuple. You have some dicts that appear to be using complex() instances as a pragmatic, but somewhat sneaky, mechanism to carry around two values (X and Y). Just use a tuple. Since tuples are hashable, they can be used as dict keys. In terms of Python syntax, a tuple is created by the comma operator, not the parentheses (the latter are used mainly for readability and precedence). In your situation, that means a dict keyed by tuple has a nice syntax: for example, self.cells[dx, dy] = Cell(dx, dy).

Just use a stack. Based on variable naming, your algorithm in back_tracker() claims to be using a stack, but it's not a stack; it's a dict. As a result, you end up needing awkward push/pop mechanics where you're always checking length, which leads you to write two unnecessary helper functions. Just use a regular Python list, which behaves like a stack for your purposes of pushing (append) and popping (pop).

Python collections have sensible boolean behavior. To check whether a collection is empty, just evaluate it for truth: while stack. No need to deal with length checks for such situations.

Prefer collections over a proliferation of single variables. In list_unvisited(), you create 4 separate variables for each neighbor position. Although those variables are well-named and serve a nice documentation purpose, that fateful decision forces you to invent the add_if_unvisited() utility function to perform the logic for each variable. While it's good that you're not writing repetitive code, I think you'd be better served by just creating a collection of positions and iterating over it to build the list of valid neighbors. You can achieve the same readability benefits of the 4 separate variable names via simple comments. However, even better would be to define Maze.NEIGHBOR_OFFSETS and just iterate over those offsets. Here's the idea:

class Maze:
 # A good use case for a class-level attribute.
 NEIGHBOR_OFFSETS = (
 (0, -1), # north
 (1, 0), # east
 (0, 1), # south
 (-1, 0), # west
 )
 def unvisited_neighbors(self, current):
 x, y = (current.x_pos, current.y_pos)
 neighbors = []
 for dx, dy in self.NEIGHBOR_OFFSETS:
 pos = (x + dx, y + dy)
 if pos in self.cells:
 current = self.cells[pos]
 if current.visited == 0:
 neighbors.append(current)
 return neighbors

• Martin Frank: wow thank you very much - all things you told me were new to me doing python... Especially the mis-useage of the type (using stack instead of dict) since i simply do not know enough python to do it better! - and yes, thanks for the NEIGHBOR_OFFSET, i am used to use enums (or even a separate class) for that in java. i think i'll have to read about initialization on python as well!!!
• Martin Frank: readable.com/blog/joining-the-readable-io-team is this "join Team Readability"??
• Martin Frank: big wow effekt after adepting your changes - funny what is possible with python :-) thanks again!