ASCII art minesweeper clone

Question

I wanted to code a small ASCII art minesweeper clone, to see what it looked like and how much code it needed.

The result is not so bad, under 80 new lines of code in total. It looks kinda readable to me, but I wrote it just know, so I'm biased.

Input checking...

I feel like I got pretty much obsessed with input checking, so much that entering garbage will always produce nice error reports to the user and not crash.

Parsing coordinates is a worthy effort

Validating and parsing coordinates of the form LETTER + NUMBER is a pain, but I feel like that input format is user friendly, so it is going to remain that way.

Separation of concerns

The interaction with the outside world is all inside the main minesweeper function, because mixing concerns of logic and interaction together makes the code unreadable.

Doctests

The code has some doctests, as I feel like seeing an example usage of a function makes the code easier to read (apart from giving a sanity check).

Re-use

I re-used some of my old code, a module about the game of life and a module about the user interaction for this. You can find them in a gist.

import doctest
import string
import random
import life
from user_interface import general_input
MINE = -1
def minesweeper():
 size = general_input("The size of the board? ", type_=int, set_=range(1, 9+1))
 mines = general_input("The number of mines? ", type_=int, set_=range(1, size*size+1))
 board = random_mineswepper_board(size, mines)
 already_seen = set()
 while True:
 print(''.join(show_board(board, already_seen)))
 raw_coordinates = general_input(
 "Enter the coordinates: ",
 costum_validation = lambda coord: is_coordinate_input_valid(coord, board),
 costum_validation_error = """
Coordinate must be in the form LETTER + NUMBER
And must not exceed the size of the board.
 Example: B4
"""
 )
 x, y = parse_coordinates(raw_coordinates)
 already_seen.add((x,y))
 if is_mine(x, y, board):
 print("You hit a mine!\nGame Over|")
 print(''.join(show_board(board, already_seen)))
 break
def is_coordinate_input_valid(coords, board):
 """
 >>> is_coordinate_input_valid('FoooBar', random_mineswepper_board(3, 1))
 False
 >>> is_coordinate_input_valid('C2', random_mineswepper_board(5, 2))
 True
 >>> is_coordinate_input_valid('Z4', random_mineswepper_board(3, 2)) # Out of bounds
 False
 """
 return len(coords) == 2 and \
 coords[0].lower() in string.ascii_lowercase and \
 coords[1] in string.digits and \
 string.ascii_lowercase.index(coords[0].lower()) < len(board) and \
 int(coords[1]) <= len(board)
def random_mineswepper_board(size, number_of_mines):
 """
 >>> random.seed(0)
 >>> random_mineswepper_board(3, 2)
 [[1, 1, 0], [-1, 2, 1], [1, 2, -1]]
 """
 board = [ [0 for _ in range(size)] for _ in range(size)]
 mined = set()
 for _ in range(number_of_mines):
 x, y = random.randint(0, len(board) - 1), random.randint(0, len(board) - 1)
 while (x, y) in mined:
 x, y = random.randint(0, len(board) - 1), random.randint(0, len(board) - 1)
 board[x][y] = MINE
 mined.add((x,y))
 return list(life.in_chunks_of(size,
 [MINE if cell == MINE else nears.count(MINE)
 for cell, nears in life.with_neightbors(board)]))
def show_board(board, already_seen, sep = ' | ', cheat=False):
 """
 >>> print(''.join(show_board([[2,2,1],[-1,-1,1],[2,2,1]], {}, cheat=True)))
 | A | B | C | 
 1 | 2 | 2 | 1 | 
 2 | M | M | 1 | 
 3 | 2 | 2 | 1 | 
 <BLANKLINE>
 """
 print(' ' + sep + sep.join(string.ascii_uppercase[i] for i in range(0, len(board))) + sep)
 for x, _ in enumerate(board):
 yield str(x + 1) + sep
 for y, _ in enumerate(board[0]):
 if True if cheat else (x, y) in already_seen:
 yield 'M' if board[x][y] == MINE else str(board[x][y])
 else:
 yield 'X'
 yield sep
 yield '\n'
def parse_coordinates(coords):
 """
 >>> parse_coordinates('C2')
 (1, 2)
 """
 return (int(coords[1]) - 1, string.ascii_lowercase.index(coords[0].lower()))
def is_mine(x, y, board):
 """
 >>> is_mine(0, 0, [[MINE]])
 True
 """
 return board[x][y] == MINE
if __name__ == "__main__":
 doctest.testmod()
 random.seed()
 minesweeper()

• Simon Forsberg: How does the actual ascii-art output look?
• Caridorc: @SimonForsberg well, it is just a grid, not really "art" -> gist.github.com/CAridorc/e6e72dd4067f84e2f79b

Answer 1

It does look kind readable, but there are points I would like to address, in addition to most of the points by codesparkle.

• Please do use print function - Even though Python 2.7, I think it's better to use the print function to ease the bridge within Python 3 and Python 2. You could however use the future import
• What is general_input()? – This sounds like an interesting function, which would be nice to see (and required to get your code to work). (Added: The general_input is found here, and does provide some interesting reading.)
• Complete show_board() – It's kind of strange to see the print( ''.join(...)) construct calling show_board(). I think you should move the join() within the function, which would remove the clutter from your main code. (See last point of mine)

• Strange formatting when inputting coordinates – This is a minor detail, but I get confused when reading the code where you input coordinates. The kind of loose "Enter the coordinates" I see the reasoning behind, but the following lines with long lines and breaking of indentation makes it a little hard to read. Here are some ideas to clean it up:

  • Possibly use \ to allow for breaking the line and start the new line with the parameter value
  • Either have the custom validation error as a constant at top of file, or use something like textwrap.dedent to keep the text indented in your code (but still nicely output).

  Here is one variant use textwrap.dedent:

  raw_coordinates = general_input(
   "Enter the coordinates: ",
   custom_validation = \
   lambda coord: is_coordinate_input_valid(coord, board),
   custom_validation_error = textwrap.dedent(
   """Coordinate must be in the form LETTER + NUMBER
   And must not exceed the size of the board.
   Example: B4
   """
   )
  

  Using the dedent function has the added benefit of making the ending parentheses, ), pop out a little more and thusly helping to clarify the code block. I would also consider changing the costum_validation to either custom_validation or even simpler validation. No need to specify that it is custom, as if it is present you already know it is specific...

• Join is_coordinates_valid_input and parse_coordinates – To have a parse coordinates that doesn't validate it's input is kind of strange, and at the same time to duplicate the logic is bad. My suggestions is therefore to convert the coordinates within parse_coordinates and simplify the is_coordinates_valid_input into something like the following (using size of the board, and not the board itself):

  import string
  import doctest
  def is_coordinates(coords, board_size):
   """Verifies that the coords is actual coordinates within the board.
   >>> is_coordinates('C4', 5)
   True
   >>> is_coordinates('Z99', 100)
   True
   >>> is_coordinates('Hi there', 'Bye')
   False
   >>> is_coordinates('Z99', 5)
   False
   """ 
   try:
   row, col = parse_coordinates(coords, board_size)
   return True
   except ValueError:
   return False
  def parse_coordinates(coords, board_size):
   """Converts text to coordinate pair, and verifies legality.
   >>> parse_coordinates('C4', 5)
   (3, 2)
   >>> parse_coordinates('Z99', 100)
   (98, 25)
   >>> parse_coordinates('Hi there', 'Bye')
   Traceback (most recent call last):
   ValueError: Coordinates needs to be in the form LETTER + NUMBER, i.e. 'C4'.
   >>> parse_coordinates('Z99', 5)
   Traceback (most recent call last):
   ValueError: Coordinates are outside of board.
   """ 
   try:
   col = string.ascii_lowercase.index(coords[0].lower())
   row = int(coords[1:]) - 1
   except (IndexError, ValueError):
   raise ValueError("Coordinates needs to be in the form LETTER + NUMBER, i.e. 'C4'.")
   if not ( 0 < row < board_size and 0 < col < board_size):
   raise ValueError("Coordinates are outside of board.")
   return row, col
  

  The actual code is short, but the doctests are longer... However this code only use relies on one convertion, not two like in your code. And you get the added benefit of actually verifying the coordinates when parsing them.

• Bug/feature: It is not possible to win?! – In your game you can only loose by hitting a mine. You can't not win and exit the game...

• Feature missing: I would love for a method to place a flag – I miss the opportunity to actually place a flag on where I believe a mine exists. In your code I have to remember all the mines I've found

• Use random.randrange(size) instead of random.randint(0, len(board) - 1) - The former should return the same range of random numbers, but it reads easier and shortens the code. Or even size - 1 is better than needlessly calculating the len(board) multiple times.

• Feature? Doctest on random creation of board?! – If this test actually works, doesn't that mean your random creation is not very random? Does your call to random.seed() after the doctest.testmod() salvage this one?

• Use list comprehension and a inner function on show_board() - In addition to points made my codesparkle, related to use of for loops and enumerate I would consider changing this into list comprehensions directly, and/or possibly use an inner function so that you could make the show_board() actually output the board as the function name indicates. This last part can be done like the following:

  def show_board(board, already_seen, sep = ' | ', cheat=False):
   """
   >>> show_board([[2,2,1],[-1,-1,1],[2,2,1]], {}, cheat=True)
   | A | B | C | 
   1 | 2 | 2 | 1 | 
   2 | M | M | 1 | 
   3 | 2 | 2 | 1 | 
   <BLANKLINE>
   """
   def _show_board():
   print(' ' + sep + sep.join(string.ascii_uppercase[i] for i in range(0, len(board))) + sep)
   for x, _ in enumerate(board):
   yield str(x + 1) + sep
   for y, _ in enumerate(board[0]):
   if True if cheat else (x, y) in already_seen:
   yield 'M' if board[x][y] == MINE else str(board[x][y])
   else:
   yield 'X'
   yield sep
   yield '\n'
   print(''.join(_show_board()))
  

  Notice how the inner function uses the same scope as the outer function, so you don't need to pass all parameters. And this is actually considered good style, and not counted as using globals. Using this you can now call show_board(...) in your main code, and it automatically does the join and print by it self.

  By the way, there is a feature that fails the doctest as you hard code 'M' within show_board() when cheat = True. Go with the other suggestion of using MINE = '*' and use that as the yielded value.

On a final note, I think you've done quite a lot of nice stuff here, but there are room for improvements and clarifications as suggested above. I haven't gotten around to implementing your special interfaces, and as such I haven't actually managed to test all of it. So code presented is untested, but you should get the gist of it.

• Caridorc: Take your time :) By the way I like this review, in-depth as always.

Answer 2

Overall, your code is quite good. But there's always room for improvement:

Syntax & Spelling

• Your question is tagged as python-2.7 so use print as a statement rather than as a function, or, better yet, use from __future__ import print_function to gain access to this feature.

• Indexing with magic numbers, such as coords[1], is ugly. Prefer sequence unpacking: col, row = coords

• Overuse of the ternary operator:

  if True if cheat else (x, y) in already_seen:
  

  This is probably the most confusing line of Python code I have ever come across. Equivalent to:

  if cheat or (x, y) in already_seen:
  

  Which would you rather read when trying to comprehend someone else's code?

• Use xrange instead of range unless you need to keep the list

• custom, not costum

• neighbors, not neightbors

Structure & Logic

• Overall, this seems well structured, with sensible use of functions. However, is_mine is only used once and should probably be inlined—here, the function does not provide a higher level of abstraction than just writing out the code: board[x][y] == MINE

• Your coordinate validation can be simplified. Instead of scanning through all these lists, use the convenience methods defined in str:

  try:
   col, row = coords
   return (col.isalpha() and
   row.isdigit() and
   col.upper() < string.ascii_uppercase[board_size] and
   int(row) <= board_size)
  except ValueError:
   return False
  

  Notice that I replaced board with board_size in is_coordinate_input_valid. There's no need to pass in the whole board when all we need is its size.

• show_board does not actually print the board, so it should be renamed to something like format_board. In it, you do this twice:

  for x, _ in enumerate(board):
  

  Why bother using enumerate when you through away the second element of the result? Just use

  for x in xrange(len(board)):
  

• If MINE = 'M' instead of MINE = -1, then you could avoid another ternary expression when printing the board: yield str(board[x][y])

• There's no need to call random.seed at the bottom.

Apart from these (mostly minor) issues, the code is easy to follow. Kudos for using doctests!

• SuperBiasedMan: "use print as a statement rather than as a function" - I don't hear people suggest this often. People usually say the opposite because of compatability with Python 3, so what's the reason for your suggestion?
• Adam: @SuperBiasedMan I, too, prefer the print function. The original code pretends to use print as a function, masking the fact that it is a statement in Python 2.x — later on, someone might try to pass an additional argument (such as sep) and fail. The best choice would indeed be to import the print function from __future__. Failing that, at least the statement should be used properly. I am editing my answer to communicate this better.