Displaying a Sudoku Board

Question

For my programming class, we have been tasked with creating a Sudoku solver; the first step of the project was to create a program which would display the game board based on input provided by the user.

import math #To be used later


def board_filler():
    """Creates the sudoku board from user input"""
    board = [[], [], [], [], [], [], [], [], [], [], []]
    for x in enumerate(board):
        #If it is one of the rows that have lines, add them
        if ((x[0] + 1) % 4) == 0:
           for y in range(11):
               board[x[0]].append("-")
        else:
            for y in range(11):
                #If it is a column that has lines in it, add them
                if ((y + 1) % 4) == 0:
                    board[x[0]].append("|")
                else:
                    #Repeat until an inout has been entered
                    z = True
                    while z:
                        z = False
                        if x[0] > 7:
                            xRead = x[0] - 1
                        elif x[0] > 3:
                            xRead = x[0]
                        else:
                            xRead = x[0] + 1
                        if y > 7:
                            yRead = y - 1
                        elif y > 3:
                            yRead = y
                        else:
                            yRead = y + 1
                        number = input("Please enter a number for the square in column %s and in row %s, if there is no number, just hit enter:" %(xRead, yRead))
                        #Trys to make it a number, then checks to see if it is a number 1 to 9
                        try:
                            number = int(number)
                            if number > 9 or number < 1:
                                z = True
                                print("Please enter a number between 1 and 9")
                            else:
                                board[x[0]].append(number)
                        #If it is not a number, check if its empty
                        except (TypeError, ValueError):
                            #If its empty, add a space
                            if len(number) == 0:
                                board[x[0]].append(" ")
                            #If not ask for a number
                            else:
                                z = True
                                print("Please enter a number")
    return board

def board_printer(board):
    """Prints the sudoku board"""
    #Turns board into str to make the .join work
    for x in enumerate(board):
        for y in enumerate(board):
            board[x[0]][y[0]] = str(board[x[0]][y[0]])
    #Prints the board
    for x in enumerate(board):
        print(" ".join(board[x[0]]))


board_printer(board_filler())

A few notes for review:

• The board_filler function feels quite bloated.
• I don't like the way I implemented xRead and yRead.

How can I improve my code?

Answer 1

First of all, let the code speak! Don't unnecessarily add comments unless they are explaining a really complicated piece of code.

Basic definitions:

A magic number is defined to be:

Unique values with unexplained meaning or multiple occurrences which could (preferably) be replaced with named constants

Can you find the magic numbers in your code?
Yes that's right, the culprits are 11 and 4.

Let's just define variables for those!

ROWS = 11
COLS = 11
GRID_ROWS = 4
GRID_COLS = 4

Next, let's replace every magic number with the appropriate variable!

Making the code shorter!

Function board_printer

Why use enumerate? Why make everything a str?

Your whole function can just be rewritten as the following:

def board_printer(board):
    """Prints the sudoku board"""

    for row in board:
        print(*row)

Function board_filler

You don't need to use for x in enumerate(board) at all!
Just use for x in range(ROWS) and change every instance of x[0] to x accordingly

---

Instead of

if ((x[0] + 1) % GRID_ROWS) == 0:
    for y in range(11):
        board[x].append("-")

Use

if ((x[0] + 1) % GRID_ROWS) == 0:
    board[x] = ["-"] * COLS

---

Use formatting!

for taking input of number, use number = input("Please enter a number for the square in row {xRead} and in column {yRead} (hit enter for no number): ") instead of using %s

---

Why use a dummy variable z?

Just do:

while True:
    number = input("Please enter a number for the square in column {x[0] + 1} and in row {y + 1} (hit enter for no number): ")

    try:
        number = int(number)

        if number > 9 or number < 1:
            raise ValueError
        else:
            board[x].append(number)

        break

    except (TypeError, ValueError):
        if not number:
            board[x].append(" ")
        else:
            print("Please enter an integer between 1 and 9")

Essentially, it tries to take an input, check if the value is an integer between 1 and 9, else raises an exception. If the number was indeed valid, it makes it to the end of the try statement, which causes the loop to break

Misc

• Run board_printer(board_filler()) inside if __name__ == __main__:. This causes board_printer(board_filler()) not to run if it was imported from another module.

• Try using list(map(int, input(f'Enter {COLS - (COLS // GRID_COLS)} space separated integers: ').split())) for each row. It is tiring to fill every single cell by hand! So ask input for a whole row. It's your personal preference though!

Hope this helps!

EDIT:

This part is to solely dedicated to removing xRead and yRead and is not related to the above improvements

First we have to change
ROWS = COLS = 11 to ROWS = COLS = 9 and GRID_ROWS = GRID_COLS = 4 to GRID_ROWS = GRID_COLS = 3

Do not append - or | to board at all!

Just remove all statements that append - or | and also remove xRead and yRead. Now, the board would look like a sudoku board without - or |

---

In the board_filler use

def board_printer(board):
    """Prints the sudoku board"""

    for row in range(ROWS):
        s = ''

        for col in range(COLS):
            s += str(board[row][col]) + ' '

            if not (col + 1) % GRID_COLS:
                s += '| '

        print(s)

        if not (row + 1) % GRID_ROWS:
            print('-' * len(s))

This will print - or | according to the row or column!

---

Final code including all above mentioned improvements

ROWS = COLS = 9
GRID_ROWS = GRID_COLS = 3

def board_filler():
    """Creates the sudoku board from user input"""

    board = [[] for _ in range(ROWS)]

    for x in range(ROWS):
        for y in range(COLS):
            while True:
                number = input(f"Please enter an integer for the square in column {x + 1} and in row {y + 1} (hit enter for no number): ")

                try:
                    number = int(number)

                    if number > 9 or number < 1:
                        raise ValueError
                    else:
                        board[x].append(number)

                    break

                except (TypeError, ValueError):
                    if not number:
                        board[x].append(" ")
                    else:
                        print("Please enter an integer between 1 and 9")

    return board

def board_printer(board):
    """Prints the sudoku board"""

    print()

    for row in range(ROWS):
        s = ''

        for col in range(COLS):
            s += str(board[row][col]) + ' '

            if not (col + 1) % GRID_COLS:
                s += '| '

        s = s[:-1] # Removes trailing space

        print(s)

        if not (row + 1) % GRID_ROWS:
            print('-' * len(s))

if __name__ == '__main__':
    board_printer(board_filler())

NOTE: board_filler doesn't look as bloated either! The code kills 2 birds with one stone!

Answer 2

I'm going to take a pass through board_filler as if I were cleaning up my own code, take notes on everything I changed, and share the final result.

1. Adding type hints so mypy will tell me if I mess up during any of this refactoring.
2. Oh weird, my board isn't a list of list of ints like I thought, it's also got strings in it? I'll go ahead and add the typing, but I should come back to that because it feels like it might be a clue the data model is a little confused.
3. Reassigning number(string) to number(int) is making mypy complain, so I'll just move that input inside the int cast for now and take a closer look later at whether that makes sense.
4. No point using enumerate if I'm only interested in the index; I'll change it so I'm just iterating over the range of indices. Oh look, mypy helps me catch all the places I need to change x[0] to just x!
5. Looks like we have the length of board hardcoded in a couple of places; I'm just going to change those to len(board) so that if we adjust board the rest of the code will just magically work.
6. This % 4 is troubling. Let's just define GRID = 4 and use that.
7. Wow, I'm not even sure what the rest of this code is doing. Let's see if cutting down the indentation and adding linebreaks between "paragraphs" helps make it easier to follow...

At this point my code looks like:

from typing import List, Union

GRID = 4  # draw a grid line every 4 rows/cols

def board_filler() -> List[List[Union[int, str]]]:
    """Creates the sudoku board from user input"""
    board: List[List[Union[int, str]]] = [[], [], [], [], [], [], [], [], [], [], []]
    for x in range(len(board)):

        #If it is one of the rows that have lines, add them
        if ((x + 1) % GRID) == 0:
           for y in range(len(board)):
               board[x].append("-")
            continue

        for y in range(len(board)):

            #If it is a column that has lines in it, add them
            if ((y + 1) % GRID) == 0:
                board[x].append("|")
                continue

            #Repeat until an input has been entered
            z = True
            while z:
                z = False

                if x > 7:
                    xRead = x - 1
                elif x > 3:
                    xRead = x
                else:
                    xRead = x + 1

                if y > 7:
                    yRead = y - 1
                elif y > 3:
                    yRead = y
                else:
                    yRead = y + 1

                #Tries to make it a number, then checks to see if it is a number 1 to 9
                try:
                    number = int(input(
                        "Please enter a number for the square in column %s and in row %s, if there is no number, just hit enter:" 
                        % (xRead, yRead)
                    ))
                    if number > 9 or number < 1:
                        z = True
                        print("Please enter a number between 1 and 9")
                    else:
                        board[x].append(number)
                #If it is not a number, check if its empty
                except (TypeError, ValueError):
                    #If its empty, add a space
                    if not number:
                        board[x].append(" ")
                    #If not ask for a number
                    else:
                        z = True
                        print("Please enter a number")
    return board

It is now apparent to me that a huge part of the complexity of this function is due to the fact that we're mixing our actual data (the numbers) with the display logistics (drawing the grid lines). That's going to be a continuous source of pain (what we call in the biz "technical debt"), especially if we plan to write code later that tries to actually solve the puzzle. What if we just took all that grid stuff out and let board_filler return a List[List[int]]?

It turns out that that makes the code a LOT simpler, and now all the complexity of board_filler is very clearly in getting the user input. Let's just break that out into its own function...

def get_number_for_square(x: int, y: int, max: int) -> Optional[int]:
    """Prompt the user for a number between 1 and max until they give
    us one or just give us a blank line (in which case return None)."""
    choice = input(
        ("Please enter a number for the square in column %s and in row %s" +
         "; if there is no number, just hit enter: ") % (x, y)
    )
    if len(choice) == 0:
        return None
    try:
        number = int(choice)
        assert 1 <= number <= max
        return number
    except:
        print("Please enter a number between 1 and %d." % max)
        return get_number_for_square(x, y, max)

Between that and getting rid of the grid stuff, board_filler is suddenly quite a bit smaller. Using len(board) everywhere is starting to bother me; since we're constructing the board inside this function, let's just take the size as a parameter and construct the board to match the size, rather than building it the other way around.

def board_filler(size: int = 9) -> List[List[Optional[int]]]:
    """Creates the sudoku board from user input"""
    board: List[List[Optional[int]]] = []
    for x in range(size):
        board.append([])
        for y in range(size):
            board[x].append(get_number_for_square(x, y, size))
    return board

Now all I need to do is replace the grid drawing logic that I chopped out. Since that has to do with how we're displaying the board, not the value of the board itself, it belongs in my board_printer function. As with the size magic number, let's make that a parameter, because why not:

def board_printer(board: List[List[Optional[int]]], grid_size: int = 3) -> None:
    """Pretty-prints the board, with grid lines every grid_size squares."""
    for y in range(len(board)):
        if y % grid_size == 0 and y > 0:
            print("-" * (len(board) + (math.ceil(len(board) / grid_size)) - 1))
        for x in range(len(board)):
            if x == len(board) - 1:
                end = "\n"
            elif (x + 1) % grid_size == 0:
                end = "|"
            else:
                end = ""
            print(board[y][x] if board[y][x] else " ", end=end)

There are probably more graceful ways of doing that printing, but it's nice and flexible if we decide we want to change up how the grid is printed. At the end of it, I can still do:

board_printer(board_filler())

and I think it still does about what the original code did, but hopefully this version is easier to follow (and it's much more flexible now)!

Answer 3

board_filler() feels bloated, because it is trying to do 2 unrelated things: 1) building some of the graphics for displaying the board, and 2) getting input for the numbers in the board. These should be split into separate functions.

Asking the user to enter 81 values or blank lines is a poor user experience. It would be easy to loose their place and enter the wrong data. Consider letting the user enter the data a row at a time, using a '-' for blank spaces in the game board (and ignore blanks in the input). All of these would be valid:

Enter row 1: --89-1---
Enter row 2: 19- 2-- ---
Enter row 3: - 5 -  - 7 - - - 8
etc.

The code would look something like:

def get_board(nrows):
    print("Enter the grid one row at a time.  Use '-' for blank spaces.\n")

    rows = []
    for n in range(nrows):
        row = input(f"Enter row {n}: ")
        rows.append(row.replace(' ', '')

    return rows

A function like board_filler() could then take the list of strings returned by get_board() to fill up the sudoku grid. This separation of concerns (one function to get input and another to fill the grid) makes it easy to make changes. For example, board_filler() wouldn't care where the grid data came from as long as it was a list of strings. It could easily be function that read a grid from a file or scraped it from a web site.

There isn't any reason to convert the numbers in the grid into int's. They aren't be used for their numerical value (they aren't being added or anything). They are just unique symbols. That way everything in the grid is a character.