patternpythonMinor
Python 3 Minesweeper
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':
```
#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
So instead of your original:
Applying these four comments to your first function
Improve text handling
You do loads of string concatenation using
Here is an example on how to do this in
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
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,
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:
-
Use the
Some issues related to the highscore list
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_casefor 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 notshowgrid(),numberofsurrounding()and so on... Be consistent is the key here!
- Add vertical spacing around code blocks - I tend to insert blank lines after
forandwhileloops andifstatements, 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
a1fto flag thea1cell.
- 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 callplay()and the game ends, it returns to this level, and then you ask forreplay().
-
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.datfile yet... This by the way, yet another magic name, which should have been a constant at the top.
- You hide
timewithin thescore()function – In my environment this breaks stuff. You can't usetimeas 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.