# Check_sudoku in Python

I have just started to learn programming with Python. I have been going through several online classes for the last month or two. Please bear with me if my questions are noobish.

One of the classes I am taking asked us to solve this problem.

# Define a procedure, check_sudoku,
# that takes as input a square list
# of lists representing an n x n
# sudoku puzzle solution and returns the boolean
# True if the input is a valid
# sudoku square and returns the boolean False
# otherwise.

# A valid sudoku square satisfies these
# two properties:

#   1. Each column of the square contains
#       each of the whole numbers from 1 to n exactly once.

#   2. Each row of the square contains each
#       of the whole numbers from 1 to n exactly once.

# You may assume the the input is square and contains at
# least one row and column.


After a while of trying to come up with the best code I can, this is what I ended up with.

def check_sudoku(sudlist):
x = range(1, len(sudlist)+1) # makes a list of each number to be found
rows = [[row[i] for row in sudlist] for i in range(len(sudlist))] # assigns all the rows to a flat list
z = range(len(sudlist))
for num in x:
for pos in z:
if num not in sudlist[pos] or num not in rows[pos]:
return False
return True


This code does work and passed all checks the class asked for. I just wonder what problems could the code have or how would you improve it?

I am just trying to improve by getting input from others. I appreciate any suggestions.

-
The problem seems poorly stated. A Sudoku grid has three types of constraint: rows, columns, and blocks. A grid that only has row and column constraints is known as a Latin square. –  Gareth Rees Feb 17 '13 at 22:58

I would use set to do the checks, and you can build the columns using some zip magic:

def check_sudoku(sudlist) :
numbers = set(range(1, len(sudlist) + 1))
if (any(set(row) != numbers for row in sudlist) or
any(set(col) != numbers for col in zip(*sudlist))) :
return False
return True


I have tested it with the following samples:

a = [[1, 2, 3, 4, 5],
[2, 3, 5, 1, 4],
[3, 5, 4, 2, 1],
[4, 1, 2, 5, 3],
[5, 4, 1, 3, 2]] # latin square

b = [[1, 2, 3, 4, 5],
[2, 3, 5, 1, 4],
[3, 5, 4, 2, 1],
[4, 1, 2, 5, 3],
[3, 4, 1, 5, 2]] # 1st and 4th elements of last row interchanged

c = [[1, 2, 3, 4, 5],
[2, 4, 5, 1, 4],
[3, 5, 4, 2, 1],
[4, 1, 2, 5, 3],
[5, 3, 1, 3, 2]] #1st and 5th elements of second column intechanged

>>> check_sudoku(a)
True
>>> check_sudoku(b)
False
>>> check_sudoku(c)
False

-
+1 for zip(*z)! –  Yulong Feb 5 '14 at 23:47
def check_sudoku(sudlist):
x = range(1, len(sudlist)+1) # makes a list of each number to be found'


Avoid storing things in single-letter variable names. It just makes it hearder to folllow your code

    rows = [[row[i] for row in sudlist] for i in range(len(sudlist))] # assigns all the rows to a flat list


You could actually do rows = zip(*sudlist) which will do the same thing. Admittedly, if you've not seen the trick before its non-obvious.

    z = range(len(sudlist))
for num in x:


There's not really any reason to store the list in x, and then iterate over it later. Just combine the lines.

        for pos in z:


Instead use for column, row in zip(sudlist, rows): and you won't need to use the pos index

            if num not in sudlist[pos] or num not in rows[pos]:
return False
return True

-
You're right that the OP's variable names are poorly chosen, but I don't think that "avoid single-letter names" is quite the right criterion. What names would you prefer here: def quadratic(a, b, c): return [(-b + sign * sqrt(b ** 2 - 4 * a * c)) / (2 * a) for sign in (-1,+1)] –  Gareth Rees Feb 18 '13 at 11:00
@GarethRees, agreed that its not absolute, that's why I said "avoid" not "never do this" –  Winston Ewert Feb 18 '13 at 15:08