Skip to main content
@dataclass
class Cell:
    revealed: bool = False
    has_mine: bool = False
    has_flag: bool = False
    neighbouring_mines: int = 0

    def is_blank(self) -> bool:
        return not self.has_bombhas_mine and self.neighbouring_bombsneighbouring_mines == 0
@dataclass
class Cell:
    revealed: bool = False
    has_mine: bool = False
    has_flag: bool = False
    neighbouring_mines: int = 0

    def is_blank(self) -> bool:
        return not self.has_bomb and self.neighbouring_bombs == 0
@dataclass
class Cell:
    revealed: bool = False
    has_mine: bool = False
    has_flag: bool = False
    neighbouring_mines: int = 0

    def is_blank(self) -> bool:
        return not self.has_mine and self.neighbouring_mines == 0
Source Link
Jasmijn
  • 426
  • 2
  • 7

(I've taken the liberty of converting all identifiers to PEP8 style.)

Board state

The state of a cell on a board is encoded with a single integer, which combines the following information:

  • Is this cell revealed?
  • Does this cell have a mine?
  • Does this cell have a flag?
  • How many neighbours of this cell are mines?

This results in complicated code to check those properties, numerous magic numbers, and a lot of crevices where bugs can creep in. On the upside, it's fairly space efficient, but unless you're planning on allowing giant boards, that shouldn't make much of a difference. Here's my proposal:

@dataclass
class Cell:
    revealed: bool = False
    has_mine: bool = False
    has_flag: bool = False
    neighbouring_mines: int = 0

    def is_blank(self) -> bool:
        return not self.has_bomb and self.neighbouring_bombs == 0

(I like using dataclasses for things like this, but of course there are plenty of other options, like attrs or a plain Python class!)

This allows you to make various MineBoard methods less complex, for example:

def toggle_flag(self, row, col):
    self.board[row][col].has_flag = not self.board[row][col].has_flag

allocate_mines

In all other places, you use row and column indexing, but in this method you're using an index. For consistency, I'd use a list of tuples for the mine locations. Additionally, you don't need to generate this list yourself, you can use random.sample:

def allocate_mines(self, width, height, number_of_mines):
    all_indices = [(row, col) for row in range(height) for col in range(width)]
    mines = random.sample(all_indices, number_of_mines)
    for row, col in mines:
        self.set_mine(row, col)
        self.set_adjacent_mines(row, col)

is_valid_cell

You're using this method in several places inside loops. Instead of looping unnecessarily over out-of-bound cells, try instead adjusting the range boundaries:

def set_adjacent_mines(self, row, col):
    for dr in range(max(0, row - 1), min(self.height, row + 2)):
        for dc in range(max(0, col - 1), min(self.width, col + 2)):
            self.board[dr][dc].neighbouring_mines += 1

Item access

This is just a spur-of-the-moment idea, but you could implement __getitem__ for the MineBoard class. It could access Cell objects and -- when passed slices --- could even return an iterable over the Cells. For example:

def toggle_flag(self, row, col):
    self[row, col].has_flag = not self[row, col].has_flag

def set_adjacent_mines(self, row, col):
    for cell in self[row - 1:row + 2, col - 1:col + 2]:
        cell.neighbouring_mines += 1

... Suitable implementation of __getitem__ left as an exercise for the reader.