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

Python 3 Minesweeper

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

Problem

I just want some opinions/tips on how to improve.

```
#import
import string
import random
import time
import pickle

#create grid
def create_grid(size,lastcell,numberofmines):
grid = []
for i in range(size):
row = ['0']*size
grid.append(row)
mines = create_mines(grid,lastcell,numberofmines,size)
p = surrounding(grid,size)
p.numberofsurrounding(grid,size)
return (grid,mines)

#show the grid
def showgrid(grid,size):
horizontal = ' -'+size*'----'
collum = ' '
#writes the collmum numbers
for i in string.ascii_uppercase[:size]:
collum += (i+ ' ')
print (collum,'\n',horizontal)
# writes row numbers
for idx,i in enumerate(grid, start=1):
row = str(idx)
row += '|'
for j in i:
row = row+' '+j+' |'
print (row+'\n'+horizontal)

#generated random cordinates
def generate_cordinate(size):
a = random.randint(0,size-1)
b = random.randint(0,size-1)
return (a,b)

#class fro surrounding
class surrounding(object):
def __init__(self,grid,size):
self.grid = grid
self.size = size

#creates a list with the surrounding cell for every cell
def surrounding_cells(self,row_num,col_num,size):
surronding = []
for i in range(-1,2):
for j in range(-1,2):
if i == 0 and j == 0:
continue
elif -110:
name('Pick a username:')
return username

#mainprogramme
def play():
username = name('pick a username(1-10signs):')
size,numberofmines = pickvalues()
start_time = time.time()
showngrid = [['-' for i in range(size)] for i in range(size)] #skapar en kopia av spelplanen utan mines
showgrid(showngrid,size)
first_round = True
flags = []
while True:
while True:
flag = False
lastcell = input('pick cell: ')
try:
if lastcell[2].lower() == 'f':

Solution

I tried playing your game, and neither in Python 2 (after some fiddling), nor in Python 3 can I get the game logic to work properly. So it does have some bugs here and there!

Naming and spacing

  • Add space after comma and around operators in your code – This would open up your code, and make it clearer to understand.



  • Use snake_case for variable and function names – Don'tjameverythingtogether! You don't do when writing normally, don't do it when you code either. create_grid() is good, but not showgrid(), numberofsurrounding() and so on... Be consistent is the key here!



  • Add vertical spacing around code blocks - I tend to insert blank lines after for and while loops and if statements, to help separate the logical parts of the code



  • Use docstrings to describe the purpose of the function - Instead of having a comment in front of the function, use docstrings which is both according to style and helps modern IDEs to provide some help on your functions.



So instead of your original:

#create grid
def create_grid(size,lastcell,numberofmines):
   grid = []
   for i in range(size):
       row = ['0']*size
       grid.append(row)
   mines = create_mines(grid,lastcell,numberofmines,size)
   p = surrounding(grid,size)
   p.numberofsurrounding(grid,size)
   return (grid,mines)


Applying these four comments to your first function create_grid() results in:

def create_grid(size, lastcell, number_of_mines):
    """Initialize the grid with mines, and return grid and mine posistions."""

    grid = []
    for i in range(size):
        row = ['0']*size
        grid.append(row)

    mines = create_mines(grid, lastcell, number_of_mines, size)
    p = surrounding(grid, size)
    p.number_of_surrounding(grid,size)
    return (grid, mines)


Improve text handling

You do loads of string concatenation using +, which isn't really a good way to build strings. You are better off using string.format, and make patterns for entire lines at the time.

Here is an example on how to do this in show_grid(grid, size):

def show_grid(grid, size):
    """Print the full grid with headers and dividers."""

    horizontal_line = ' -{}'.format('-----'*size)    
    ROW_PATTERN = ' {:>} | {} |'

    print('     {}'.format('   '.join(string.ascii_uppercase[:size])))  
    print(horizontal_line)

    # Write rows
    for idx, gridrow in enumerate(grid, start=1):
        print(ROW_PATTERN.format(idx, ' | '.join(gridrow)))
        print(horizontal_line)


No loosing or winning

In the code as it stands there isn't any possiblity to win or loose, but this is due to editing when posting to Code Review. But it does illustrate a good point on magic number or words. In your code you had a lot of comments in Swedish, but you also used Förlust (i.e. "Game over" or "Loss") and Vinst (i.e. "Victory") as state indicators in result(). The only thing is, you translated these into English in play() and controll(), but not in result().

Ergo it is not possible to win or loose in your code as it stands now. In general it is better either to use booleans, True or False, or constants declared at the top of the code. Then you could have changed translated them easily, and not worry if you caught all the places it were used.

General game logic

Removing some post inflicted bugs, it is possible to play, but there are few issues related to general game logic and handling:

  • How do you flag mines? – Or your help section isn't very helpful. I had to read and search in the code to find that I could type a1f to flag the a1 cell.



  • How do you abort a game? - Given that the win/loose didn't work as expected I got stuck in a loop where I wasn't able to end the game, and had to resort to breaking the program flow. It would be good to have an option to quit at any time.



  • Recursive next game and end game logic – If I win or loose (in a working version), you trigger the replay() function, which in turns starts over again. This nests deeper, and deeper. It would be better to have a main loop at top level, so that when you call play() and the game ends, it returns to this level, and then you ask for replay().



-
Use the if __name__ == '__main__': construct – It's good that you've made a mainmenu() function, it would be even better if this was within the named construct, as this would allow for your script to be used as a module as well as being run from the command line. I.e. do:

if __name__ == '__main__':
    mainmenu()


Some issues related to the highscore list

  • If you start of by displaying the highscore list it fails – I started testing to see the highscore list, and it failed as I don't have any high_scorelist.dat file yet... This by the way, yet another magic name, which should have been a constant at the top.



  • You hide time within the score() function – In my environment this breaks stuff. You can't use time as both an imported module and a local variable.



  • Every time to

Code Snippets

#create grid
def create_grid(size,lastcell,numberofmines):
   grid = []
   for i in range(size):
       row = ['0']*size
       grid.append(row)
   mines = create_mines(grid,lastcell,numberofmines,size)
   p = surrounding(grid,size)
   p.numberofsurrounding(grid,size)
   return (grid,mines)
def create_grid(size, lastcell, number_of_mines):
    """Initialize the grid with mines, and return grid and mine posistions."""

    grid = []
    for i in range(size):
        row = ['0']*size
        grid.append(row)

    mines = create_mines(grid, lastcell, number_of_mines, size)
    p = surrounding(grid, size)
    p.number_of_surrounding(grid,size)
    return (grid, mines)
def show_grid(grid, size):
    """Print the full grid with headers and dividers."""

    horizontal_line = ' -{}'.format('-----'*size)    
    ROW_PATTERN = ' {:>} | {} |'

    print('     {}'.format('   '.join(string.ascii_uppercase[:size])))  
    print(horizontal_line)

    # Write rows
    for idx, gridrow in enumerate(grid, start=1):
        print(ROW_PATTERN.format(idx, ' | '.join(gridrow)))
        print(horizontal_line)
if __name__ == '__main__':
    mainmenu()

Context

StackExchange Code Review Q#113153, answer score: 3

Revisions (0)

No revisions yet.