I have code that can generate a 4x4 Sudoku matrix by random generation of numbers. To do this, I create a Matrix4 class that can detect its elements by its rows, columns, and submatrices for a Sudoku puzzle.
The method used here is by inputting a random matrix (4x4) and then shuffle the numbers in (1,2,3,4) to be randomly put in the matrix until it became a solved Sudoku matrix.
The sudoku_gen(Matrix,f) will generate a sudoku matrix with Matrix as a Matrix4 object, and f as the frequency for the shuffle.
I intend to only use minimum number of modules: only copy and random are used here.
The weakness is that it is slow. I would appreciate feedback on this.
#### 9 January 2018,
#### Author : Arief Anbiya
#### Description : A code to generate a minimum sudoku-matrix (4x4).
#### The method used is still naive.
#### The emphasis is on the short code,
#### and only two modules used : copy and random.
#### This is my first code uploaded in GitHub
import copy
import random
def shuffle(x, freq):
y = copy.deepcopy(x);
for i in range(0,freq):
idx = int(random.uniform(0, len(x)));
val = y.pop(idx);
y.append(val);
z = copy.deepcopy(y);
return z
class Matrix4:
def __init__(self, matrix):
self.mat = copy.deepcopy(matrix);
self.rows = self.mat;
self.columns = [[i[j] for i in self.rows] for j in range(0, 4)];
sub_1 = [self.rows[0][0], self.rows[0][1], \
self.rows[1][0], self.rows[1][1]];
sub_2 = [self.rows[0][2], self.rows[0][3], \
self.rows[1][2], self.rows[1][3]];
sub_3 = [self.rows[2][0], self.rows[2][1], \
self.rows[3][0], self.rows[3][1]];
sub_4 = [self.rows[2][2], self.rows[2][3], \
self.rows[3][2], self.rows[3][3]];
self.submats = [sub_1, sub_2, sub_3, sub_4];
def show(self):
print('--Rows--');
print(self.rows);
print('--Columns--');
print(self.columns);
print('--Subgrids--');
print(self.submats);
print('--Matrix--');
for i in self.rows:
print(i);
def checking(matrix):
check = [True, True, True];
for i in matrix.rows:
if len(set(i))!=4:
check[0] = False; break
for i in matrix.columns:
if len(set(i))!=4:
check[1] = False; break
for i in matrix.submats:
if len(set(i))!=4:
check[2] = False; break
print(check);
if False in check:
return False
else:
return True
def sudoku_gen(matrix, f):
store = list(); attempt = int(0);
while not checking(matrix):
while (matrix.rows in store):
Mat = list();
for i in range(0, 4):
Y= shuffle(num, f);
while Y in Mat :
Y = shuffle(num, f);
Mat.append(Y);
matrix = Matrix4(Mat);
store.append(copy.deepcopy(matrix.rows));
attempt = attempt + 1;
print('----'+str(attempt));
return matrix
#f is the frequency of shuffeling to permute list [1, 2, 3, 4].
#This will be used in function shuffle(x, freq) and sudoku_gen(matrix, f).
f = 20;
#You may use different combinations of this parameter.
Mat = list(); num=[1,2,3,4];
for i in range(0, 4):
Y = shuffle(num, f); print(Y);
Mat.append(Y);
Matrix = Matrix4(Mat);
new_mat = list();
new_mat.append(sudoku_gen(Matrix, f));
#This is the number of sudoku-matrix to be generated.
number_of_sudoku = 5;
while len(new_mat)<number_of_sudoku:
repeat = True;
while repeat:
repeat = False;
result = sudoku_gen(Matrix, f);
#Uniqueness test:
for i in new_mat:
if i.rows == result.rows:
repeat = True; break
new_mat.append(sudoku_gen(Matrix, f));
idx = 1;
for i in new_mat:
print('***** Matrix-'+str(idx)+' *****');
idx = idx+1;
i.show();
This code is available on https://github.com/anbarief/sudoku-gen
1 Answer 1
Let's start from the top.
These are good comments:
#### 9 January 2018,
#### Author : Arief Anbiya
However, one comment character will suffice, and it is good to be explicit about what the date means:
# Created: 9 January 2018
# Author : Arief Anbiya
This is also a good comment:
#### Description : A code to generate a minimum sudoku-matrix (4x4).
But, the PEP 8 style guide recommends using a docstring at the top of the code instead:
"""
Generate a minimum Sudoku-matrix (4x4).
"""
You can add more to the docstring to summarize what the code does, how it does it, what it outputs, etc.
These comments are irrelevant and should be deleted:
#### The method used is still naive.
#### The emphasis is on the short code,
#### and only two modules used : copy and random.
#### This is my first code uploaded in GitHub
This is a good name for a function:
def shuffle(x, freq):
freq is a good name for a variable, but x is not very descriptive in this context.
PEP 8 also recommends docstrings for functions. In this case, the doc should:
- summarize what the function does
- describe the input variable types and ranges
- specify the return type
def shuffle(x, freq):
"""
Randomly shuffle a list of ...
x: This is a list of ... integers? between 0 and ?
freq: This is an integer to represent ...
Returns a list of ...
"""
You should add docstrings for all your functions.
In this line:
y = copy.deepcopy(x);
The variable name y also does not convey much meaning.
Also, there is no need for the semicolon at the end of the line.
The ruff tool
correctly identifies this poor coding style, and it also has an option
to automatically remove all the unnecessary semicolons in your code (there are many).
In the following line:
for i in range(0,freq):
it is customary to use the _ placeholder instead of the iterator
variable when that variable is not used elsewhere. Also, the default
range start is 0, which can be omitted:
for _ in range(freq):
In this line:
idx = int(random.uniform(0, len(x)));
the len(x) expression can be replaced by a variable outside of the for
loop:
num_things = len(x)
for _ in range(freq):
idx = int(random.uniform(0, num_things))
You should replace the word "things" with something applicable to your code.
This may make the code run more efficiently since len is only evaluated
once when the function is called instead of many times in the loop.
These 2 lines:
val = y.pop(idx);
y.append(val);
could be simplified as 1 line:
y.append(y.pop(idx))
This eliminates the vaguely named variable var.
Similarly, these 2 lines:
z = copy.deepcopy(y);
return z
can be:
return copy.deepcopy(y)
It's not clear to me why you need that deepcopy. You might be
able to simply:
return y
The random
module also has a method named shuffle. Investigate
whether or not your shuffle function can leverage its capabilities.
PEP 8 also recommends docstrings for classes. In this case, the doc should:
- summarize the purpose of the class
- optionally show a code example of class usage
The name of the class is a bit vague:
class Matrix4:
You could add to the name to specify what the matrix represents (Sudoku, puzzle, grid, etc.).
This is a good name for a function:
def show(self):
Consider adding a standard
__str__
function to the class. The show function could then call __str__.
This function name is a bit generic:
def checking(matrix):
Since the function returns a boolean value, it is common to use
the is_ prefix for the name, something like is_solved.
It is common to pluralize array variable names. For example:
check
would be:
checks
In this line:
for i in matrix.rows:
It is common to use i as the iterator variable in a for loop,
but it is expected to be an integer value. In this case it
would be more meaningful to name it something like:
for row in matrix.rows:
Lines like this:
check[0] = False; break
are more commonly split into 2 lines:
check[0] = False
break
ruff notifies you of this as well. The black
program can be used to automatically reformat the code for you in this case.
This line in the checking function should be deleted:
print(check);
I'm guessing it was used in the development of the code, but it is now out of place. The function should do just one thing (check for something); it should not print as well.
Statements like:
if False in check:
return False
else:
return True
are usually written without the else clause:
if False in check:
return False
return True
PEP 8 recommends using all capital letters for named constants:
f = 20;
number_of_sudoku = 5;
These could be:
FREQ = 20;
NUMBER_OF_SUDOKU = 5
Formatted strings like this:
print('----'+str(attempt));
can be simplified using f-strings:
print(f'----{attempt}')
The following:
attempt = attempt + 1;
can be simplified using the special assignment operator:
attempt += 1