Linter, style checker, static analyzer, code formatter
You should use a linter and/or a static analyzer, preferably one with an auto-correct functionality. I actually have multiple linters and multiple static analyzers configured in my editor, and they are set up so that they analyze my code while I type, and automatically correct whatever they can auto-correct when I save.
When I save your code into a file and open the file in my editor, I get a whopping
- 157(!!!) Errors
- 44 Warnings
- 21 Infos
Now, to be fair, a lot of these are duplicates, because as I mentioned, I have multiple linters and analyzers set up. Also, I have them set to pretty aggressive settings, which can sometimes be annoying and overwhelming if you work with code that you haven't freshly written yourself. (OTOH, it is tremendously helpful if you have them turned on from the start, since you will be immediately notified and can thus avoid letting the count ever get this high.)
In particular, I have type checking turned on, and almost 130 of the Errors are from Pylance complaining it can't fully determine the static type of some variable, parameter, or function. Now, if you care about static type safety or not, that is a subjective opinion, so you may ignore these Errors.
The nice thing about style checkers, linters, and static analyzers with auto-correction support, and automatic code formatters is that they do (part of) your work for you. For example, as mentioned, if I simply save your code into a file and open that file in an editor, I get 157 Errors, 44 Warnings, and 21 Infos.
If, instead, I copy&paste the code into my editor, even during the "paste" operation, it already starts automatically applying fixes, and I only get 139 Errors, 30 Warnings, and 21 Infos. And I get the code formatted according to my preferences (e.g. using " instead of ').
Consistency
Sometimes, you use two blank lines between methods, sometimes only one.
You should choose one style and stick with it. If you are editing some existing code, you should adapt your style to be the same as the existing code. If you are part of a team, you should adapt your style to match the rest of the team.
In general, if you use two different ways to write the exact same thing, the reader will think that you want to convey a message with that. So, you should only use two different ways of writing the same thing IFF you actually want to convey some extra information.
Note that PEP8 mandates two lines after classes, one line after methods and functions.
PEP8 violations
The standard community coding style for the Python community is defined in Python Enhancement Proposal 8 – Style Guide for Python Code. You should always follow the guidelines of PEP8. There are plenty of tools available that can flag and even auto-correct violations of PEP8.
Here's just a couple that my editor flagged:
- No space before colon: In some places, you have space before the colon that starts a new block. There should be no space.
- Line length: PEP8 is rather strict about line length. Some of your lines are too long.
- Sometimes, you have 1 blank line after the function. There should be 2 blank lines after a function or class.
- Docstring: All your public modules, classes, methods, and functions should have a docstring explaining their usage.
snake_case naming: The standard naming convention in Python is PascalCase for classes, snake_case for modules, functions, methods, attributes, variables, and parameters, and SCREAMING_SNAKE_CASE for constants. You are using camelCase for functions, methods, attributes, variables, and parameters. Use snake_case instead. For example, allocateMines should be allocate_mines, and so on.
Note that, if we ignore the afore-mentioned undefined types, then the naming accounts for a vast majority of the remaining issues my editor reports. So, let's fix those names.
Vertical whitespace
Your code is all bunched up together. Some empty lines would allow the code room to breathe, for example in the play function. The function is clearly separated into a series of steps: setup, game loop, finish. The same applies to the game loop itself, it also has distinct steps. Some whitespace would help draw attention to those steps:
w = int(input("Enter width of board: "))
h = int(input("Enter height of board: "))
m = int(input("Enter number of mines : "))
while m >= w * h - 1:
m = int(input("Too many mines. Enter again : "))
game = MineBoard(w, h, m)
while not game.isOver():
cls()
game.printLayout()
command = nextCommand()
splits = command.split(" ")
row = int(splits[0]) - 1
col = int(splits[1]) - 1
if command[-1] == "F":
game.flag(row, col)
else:
game.click(row, col)
game.revealAll()
cls()
game.printLayout()
if game.loseGame():
print("You lose !!")
elif game.winGame():
print("You win !!. Congradulations !!")
Actually, it would make even more sense to extract the various separate steps into separate functions.
Chained comparison
Python supports chained comparisons, i.e.
a < b and b < c
can be written as
a < b < c
You could use that here:
return row >= 0 and row < self.h and col >= 0 and col < self.w
could be
return 0 <= row < self.h and 0 <= col < self.w
Unnecessary comprehension
remains = [i for i in range(n)]
is unnecessary. You can pass any iterable to the list constructor to create a list:
remains = list(range(n))
Unused import
You import pdb but never use it. Remove the import.
Explicit inheritance from object
Your MineBoard class explicitly inherits from object. That is unnecessary in Python 3. Python 2 is no longer supported since 1 January 2020 (i.e. over 1.5 years), and Python 3 has been supported since 3 Dec 2008 (i.e. over 12.5 years). There is absolutely no reason to use Python 2 for new code in 2021.
So, your class declaration should just be
class MineBoard:
Unused variables
In one of your list comprehensions, you have unused variables:
self.board = [[0 for i in range(w)] for j in range(h)]
Neither i nor j are used. They should be called _ to make clear that they are deliberately ignored:
self.board = [[0 for _ in range(w)] for _ in range(h)]
No else / elif after return
In this piece of code
if ip > 90:
return "F"
elif ip == 9:
return "*"
elif ip == 10:
return "."
elif ip > 10:
return str(ip - 10)
return ""
All the elifs can just be ifs, since in all of the conditionals in this method, we either exit the method or the conditional was false. In other words: if we reach the if at all, we know that all the ifs before it were false, because otherwise we would already have returned from the method.
if ip > 90:
return "F"
if ip == 9:
return "*"
if ip == 10:
return "."
if ip > 10:
return str(ip - 10)
return ""
Naming
There are a couple of names in your code that could be clearer, for example ip, m, and k. In particular, it seems that the parameter k in __init__, the parameter num_of_mines in allocate_mines, and the local variable m in play mean the same thing, but the parameter k in get_random_pos does not mean the same thing as the parameter k in __init__.
They should really have more intention-revealing names. A good name should be intention-revealing. They should convey meaning.
I also noticed something strange about the MineBoard. It appears that MineBoard is not actually a board of mines. It is also a game of minesweeper. In fact, when you instantiate it, you actually assign it to a variable named game!
So, this implies two things: one, the class should probably have a different name (e.g. Game). But more importantly, the reason why it is hard to give it a proper name is that it appears to be doing too much. In particular, it represents two totally different concepts: a map / board, and a game. It should probably be split into two classes.
Mixing I/O and computation
I am not a big fan of mixing I/O and computation.
An example of what I mean is the print_layout method. It mixes responsibilities of creating the string representation and printing it. This makes it hard to reuse and hard to test. You can't just call it and check its result value in a test, for example, you actually have to capture the output from the terminal.
Instead, this method should be split into two methods. One which just creates the string representation of the board, and a second one which prints it. The first one should probably just be MineBoard's __str__ method, and the second one should probably be part of the game logic rather than the board logic.
Something like this:
def __str__(self) -> str:
return "\n".join(
[
" " * 7 + "".join(map(lambda x: "{:^7d}".format(x + 1), range(self.w))),
" " * 7 + "-" * (self.w * 7),
"\n".join(
(
"{:^7d}".format(i + 1)
+ "|"
+ " |".join(
list(map(lambda x: "{:^5s}".format(self.display(x)), row))
)
+ " | "
)
+ "\n"
+ (" " * 7 + "-" * (self.w * 7))
for (i, row) in enumerate(self.board)
),
]
)
And then in play, the two calls to game.print_layout() can simply be replaced by print(game).
Comments
Generally speaking, comments are a code smell. Mostly, comments should not exist:
- If your code is so complex that you need to explain it in a comment, you should rather try to refactor your code to be less complex so that it needs no explanation.
- If you have a comment that explains what something is, you can rather give it a name that tells the reader what the thing is.
- If you have a comment that explains how the code does something, just delete it: the code explains how the code does something.
The only acceptable thing for a comment is to explain why the code does something in a specific non-obvious way. So, for example, there is an obvious way that looks like it should work, but you tried it and it didn't work for a non-obvious reason. Then you can add a comment explaining that you are specifically using solution B even though it looks like much simpler solution A should also work, but it actually doesn't work because of issue X. Ideally, you would add a link to the pull request / code review / bug ticket where this issue is discussed in greater detail and maybe a link to a wiki page with a detailed explanation.
The major offender is this comment here:
# Convention for cell values:
# - 0 : Hidden Blank
# - 10 : Revealed Blank
# - -1 : Hidden Bomb
# - 9 : Revealed Bomb
# - 1 ~ 8 : number of adjacent bomb (hidden)
# - 11 ~ 18 : adjacent bomb (revealed)
# - x + 100 : Flagged
#
This comment is problematic for many reasons.
For one, it is placed in an awkward sport, in the middle of the class. It should probably be part of the class documentation proper, i.e. of the docstring.
However, it really should not exist at all. Such important information, and such an encoding should be encapsulated in an object. It looks like you are missing an abstraction, probably something like a Cell (which could be a namedtuple or a dataclass).
This abstraction would also allow us to move some of the methods out of MineBoard. For example, display should be an instance method of Cell. In fact, it should probably be Cell's __str__ method instead.
A big clue is the fact that you have multiple comments talking about "cells" but you have no abstraction called "cell" in your code. That is often a dead giveaway that you are missing an abstraction.
if __name__ == "__main__":
It is generally recommended to guard your main entry point using the familiar if __name__ == "__main__": construct. This way, the main entry point will only be automatically executed if the module is run as a script, but not if it is imported:
if __name__ == "__main__":
play()
Shebang line
Since you intend to run this as a script, it should have a shebang line, something like this:
#!/usr/bin/env python3
(Interim) result
#!/usr/bin/env python3
import random
from enum import auto, Enum
from subprocess import run
from textwrap import dedent
from typing import Iterable, Optional
class GameStatus(Enum):
PLAYING = auto()
LOSE = auto()
WIN = auto()
class MineBoard:
status: GameStatus = GameStatus.PLAYING
w: int
h: int
board: list[list[int]]
cells_to_open: int
def __init__(self, w: int, h: int, k: int) -> None:
# Create a new board with size w x h
self.w = w
self.h = h
self.board = [[0 for _ in range(w)] for _ in range(h)]
self.allocate_mines(w, h, k)
self.cells_to_open = w * h - k
def allocate_mines(self, w: int, h: int, num_of_mines: int) -> None:
alloc_indexes = self.get_random_pos(w * h, num_of_mines)
for i in alloc_indexes:
self.set_mine(int(i / w), i % h)
self.set_adjacent_mines(int(i / w), i % h)
def __str__(self) -> str:
return "\n".join(
[
" " * 7 + "".join(map(lambda x: "{:^7d}".format(x + 1), range(self.w))),
" " * 7 + "-" * (self.w * 7),
"\n".join(
(
"{:^7d}".format(i + 1)
+ "|"
+ " |".join(
list(map(lambda x: "{:^5s}".format(self.display(x)), row))
)
+ " | "
)
+ "\n"
+ (" " * 7 + "-" * (self.w * 7))
for (i, row) in enumerate(self.board)
),
]
)
def click(self, row: int, col: int) -> None:
value = self.reveal(row, col)
if value:
self.cells_to_open -= 1
if self.cells_to_open == 0:
self.status = GameStatus.WIN
if self.has_mine(row, col):
self.status = GameStatus.LOSE
elif self.is_blank(row, col):
for dr in range(row - 1, row + 2):
for dc in range(col - 1, col + 2):
self.click(dr, dc)
def flag(self, row: int, col: int) -> None:
if self.is_valid_cell(row, col) and self.is_hidden(row, col):
self.toggle_flag(row, col)
def is_valid_cell(self, row: int, col: int) -> bool:
return 0 <= row < self.h and 0 <= col < self.w
def get_random_pos(self, n: int, k: int) -> Iterable[int]:
res: list[int] = []
remains = list(range(n))
while k > 0:
r = random.randint(0, len(remains) - 1)
res.append(r)
del remains[r]
k -= 1
return res
# Convention for cell values:
# - 0 : Hidden Blank
# - 10 : Revealed Blank
# - -1 : Hidden Bomb
# - 9 : Revealed Bomb
# - 1 ~ 8 : number of adjacent bomb (hidden)
# - 11 ~ 18 : adjacent bomb (revealed)
# - x + 100 : Flagged
def set_mine(self, row: int, col: int) -> None:
self.board[row][col] = -1
def set_adjacent_mines(self, row: int, col: int) -> None:
for dr in range(row - 1, row + 2):
for dc in range(col - 1, col + 2):
if self.is_valid_cell(dr, dc) and not self.has_mine(dr, dc):
self.board[dr][dc] += 1
def toggle_flag(self, row: int, col: int) -> None:
if self.is_flagged(row, col):
self.board[row][col] -= 100
else:
self.board[row][col] += 100
# Open a cell and return its value
def reveal(self, row: int, col: int) -> Optional[int]:
if not self.is_valid_cell(row, col) or not self.is_hidden(row, col):
return None
if self.is_flagged(row, col):
self.toggle_flag(row, col)
self.board[row][col] += 10
return self.board[row][col]
def is_hidden(self, row: int, col: int) -> bool:
return self.board[row][col] < 9
def has_mine(self, row: int, col: int) -> bool:
return self.board[row][col] % 10 == 9
def is_blank(self, row: int, col: int) -> bool:
return self.board[row][col] % 10 == 0
def is_over(self) -> bool:
return self.win_game() or self.lose_game()
def lose_game(self) -> bool:
return self.status == GameStatus.LOSE
def win_game(self) -> bool:
return self.status == GameStatus.WIN
def is_flagged(self, row: int, col: int) -> bool:
return self.board[row][col] > 90
def reveal_all(self) -> None:
for i in range(len(self.board)):
for j in range(len(self.board[0])):
self.reveal(i, j)
def display(self, ip: int) -> str:
if ip > 90:
return "F"
if ip == 9:
return "*"
if ip == 10:
return "."
if ip > 10:
return str(ip - 10)
return ""
def cls() -> None:
run(["/usr/bin/env", "clear"], check=True)
def play() -> None:
w = int(input("Enter width of board: "))
h = int(input("Enter height of board: "))
m = int(input("Enter number of mines : "))
while m >= w * h - 1:
m = int(input("Too many mines. Enter again : "))
game = MineBoard(w, h, m)
while not game.is_over():
cls()
print(game)
command = next_command()
splits = command.split(" ")
row = int(splits[0]) - 1
col = int(splits[1]) - 1
if command[-1] == "F":
game.flag(row, col)
else:
game.click(row, col)
game.reveal_all()
cls()
print(game)
if game.lose_game():
print("You lose !!")
elif game.win_game():
print("You win !!. Congradulations !!")
def next_command() -> str:
instruction = dedent(
"""\
Commands :
<row> <col> : open cell
<row> <col> F : flag cell
q : quit
"""
)
return input(instruction).strip()
if __name__ == "__main__":
play()
Note: In order to make this answer useful for future readers, I have mostly assumed Python 3.10, which is about to be released soon. However, I don't think I have used anything that is not available in Python 3.9, and the code can be trivially made to work with at least Python 3.8.
