HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python - Minesweeper

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pythonminesweeperstackoverflow

Problem

I'm new to Python (and generally to programming), and I have recently written this Minesweeper game:

```
import random
import string
import os
class Square(object):
"""Represent a square in the cell.

mine -- if the square has a mine, it's True. Otherwise, False.
location -- a tuple that represents the (x, y) of the square in the grid.
grid -- the gird that has the square in it
"""
signs = ('*', ' ') + tuple([str(n) for n in range(1,9)])
# reserved signs - the player can't mark a square with those signs
# (signs of visible squares - it doesn't contain '.' because when the
# player mark the square with '.', he cancels the previous sign)

def __init__(self, mine, location, grid):
self.mine = mine
self.location = tuple(location)
self.grid = grid
self.sign = '.' # sign - how is the square represented to the user.
self.visible = False # not visible yet
assert self.legal_square()

def __str__(self):
return self.sign

def expose(self):
'''Make the square visible to the player (when he exposes it, for
example).'''
self.visible = True
if self.has_mine():
self.sign = '*' # The sign that means that the square has a mine
# and the player exposed it (he lost).

elif self.num() == 0: # There are no mines near the square
self.sign = ' ' # The sign that means that the square is clean,
# and there are no mines near it.

x, y = self.location[0], self.location[1]
# Expose all of the squares near the current square, and all of
# the squares near them (recursively), until we reach squares
# with numbers.
for near_x in range(x-1, x+2):
for near_y in range(y-1, y+2):
if not (near_x == x and near_y == y): # not the same square
self.grid.expose_squ

Solution

Purely from an OOP standpoint, I think that your classes don't make much sense.

Square should just be a square, it shouldn't modify any other square (as you do so in your expose function). In fact, a square doesn't even need to know it's own location (as its container should handle that) or its container. I would have a bare-bones square class with only its contents and an expose method that exposes itself and nothing else.

Grid is doing too much. It is simultaneously doing the job of a grid (working with squares) while also incorporating most of the game logic. I would move most of the game logic to another class (described below) and move the expose square logic to here. Also I think adding a tuple of tuples of Squares for the grid data structure would make sense.

Add a MineSweeper class that contains most of the actual game logic (input, redraw and etc) that is currently in Grid.

In general, your code should try to follow the single responsibility principle. That is, make each of your classes/functions responsible for one thing and one thing only. Note: this means creating functions for things like iterating over the neighbors and etc.

On the programming side:

There is no reason to generate a dummy grid and then generate a real grid. Create the real grid, and then if the first click is on a mine, move the mine somewhere else.

Your code may break if the number of mines is especially high. This is because you keep trying to generate random locations that haven't been used up in a while true: loop, which will keep your code running until the end of time if there are no spots left. I would take a look at some of the other mine generation algorithms that have been used over the years. Alternatively, you can chop your grid up into mines - 10 equal pieces, choose a random position within those sub-grids to add a mine, and finally distributing the last 10 randomly amongst the sub-grids. Take care to make sure that you aren't adding too many mines.

Context

StackExchange Code Review Q#58039, answer score: 8

Revisions (0)

No revisions yet.