patternpythonMinor
Square spiral matrix
Viewed 0 times
spiralmatrixsquare
Problem
I've written a Python script to generate the closest square spiral matrix to a given number.
I'm new to code reviews as a practice, and I'm interested in ways to improve. Please suggest improvements to the code where you see fit, particularly in regards to:
```
"""
Outputs the closest square spiral matrix of an input number
"""
from math import sqrt
def is_even_square(num):
"""True if an integer is even as well as a perfect square"""
return sqrt(num).is_integer() and (num % 2 == 0)
def find_nearest_square(num):
"""Returns the nearest even perfect square to a given integer"""
for i in range(num):
if is_even_square(num - i):
return num - i
elif is_even_square(num + i):
return num + i
def find_lower_squares(num):
"""Returns a list of even perfect squares less than a given integer"""
squares = []
for i in range(num, 3, -1):
if is_even_square(i): squares.append(i)
return squares
def nth_row(num, n):
"""Returns the nth row of the square spiral matrix"""
edge = int(sqrt(num))
squares = find_lower_squares(num)
if n == 0:
return list(range(num, num - edge, -1))
elif n >= edge - 1:
return list(range(num - 3edge + 3, num - 2edge + 3))
elif n < edge // 2:
return ([squares[1] + n] + nth_row(squares[1],n-1)
+ [num - edge - n + 1])
else:
return ([num - 3*edge + 4 + n - edge] + nth_row(squares[1],n-1)
+ [num - 2*edge + 1 - n + edge])
def generate_square_spiral(num):
"""Generates a square spiral matrix from a given integer"""
edge = int(sqrt(num))
square_spiral = [[None for x in range(edge)] for y in range(edge)]
for row in range(edge): square_spiral[row] = nth_row(num, row)
return square_spiral
def main ():
num = None
while not num:
try:
num = int(i
I'm new to code reviews as a practice, and I'm interested in ways to improve. Please suggest improvements to the code where you see fit, particularly in regards to:
- Algorithm: Is there a faster/more elegant way to generate the matrix?
- Style: I've tried to adhere to PEP8.
```
"""
Outputs the closest square spiral matrix of an input number
"""
from math import sqrt
def is_even_square(num):
"""True if an integer is even as well as a perfect square"""
return sqrt(num).is_integer() and (num % 2 == 0)
def find_nearest_square(num):
"""Returns the nearest even perfect square to a given integer"""
for i in range(num):
if is_even_square(num - i):
return num - i
elif is_even_square(num + i):
return num + i
def find_lower_squares(num):
"""Returns a list of even perfect squares less than a given integer"""
squares = []
for i in range(num, 3, -1):
if is_even_square(i): squares.append(i)
return squares
def nth_row(num, n):
"""Returns the nth row of the square spiral matrix"""
edge = int(sqrt(num))
squares = find_lower_squares(num)
if n == 0:
return list(range(num, num - edge, -1))
elif n >= edge - 1:
return list(range(num - 3edge + 3, num - 2edge + 3))
elif n < edge // 2:
return ([squares[1] + n] + nth_row(squares[1],n-1)
+ [num - edge - n + 1])
else:
return ([num - 3*edge + 4 + n - edge] + nth_row(squares[1],n-1)
+ [num - 2*edge + 1 - n + edge])
def generate_square_spiral(num):
"""Generates a square spiral matrix from a given integer"""
edge = int(sqrt(num))
square_spiral = [[None for x in range(edge)] for y in range(edge)]
for row in range(edge): square_spiral[row] = nth_row(num, row)
return square_spiral
def main ():
num = None
while not num:
try:
num = int(i
Solution
Your code is overall clean and well written.
You should use list comprehension:
Should become:
Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:
This gives double benefits:
Avoid
In mid level languages such as C you must declare array first as empty and then fill them. In high level languages such as Python you should avoid such low level behaviour and insert the values in the arrays directly:
Also we could remove the unnecessary assignment:
You should use list comprehension:
def find_lower_squares(num):
"""Returns a list of even perfect squares less than a given integer"""
squares = []
for i in range(num, 3, -1):
if is_even_square(i): squares.append(i)
return squaresShould become:
def find_lower_squares(num):
"""Returns a list of even perfect squares less than a given integer"""
return [i for i in range(num, 3, -1) if is_even_square(i)]Scientific/mathematical code like this greatly benefits from automated testing, let me show you an example:
import doctest
def find_lower_squares(num):
"""
Returns a list of even perfect squares less than a given integer
>>> find_lower_squares(40)
[36, 16, 4]
"""
return [i for i in range(num, 3, -1) if is_even_square(i)]
doctest.testmod()This gives double benefits:
- Changes to the code can be made without being worried: if you break something, the error message will show up immediately
- Code readability and documentation increase a lot, the reader can see some examples of the function being used and will understand it faster and better.
Avoid
None-important assignmentIn mid level languages such as C you must declare array first as empty and then fill them. In high level languages such as Python you should avoid such low level behaviour and insert the values in the arrays directly:
def generate_square_spiral(num):
"""
Generates a square spiral matrix from a given integer
>>> generate_square_spiral(5)
[[5, 4], [2, 3]]
>>> generate_square_spiral(17)
[[17, 16, 15, 14], [5, 4, 3, 13], [7, 1, 2, 12], [8, 9, 10, 11]]
"""
edge = int(sqrt(num))
square_spiral = [nth_row(num, row) for row in range(edge)]
return square_spiralAlso we could remove the unnecessary assignment:
edge = int(sqrt(num))
return [nth_row(num, row) for row in range(edge)]Code Snippets
def find_lower_squares(num):
"""Returns a list of even perfect squares less than a given integer"""
squares = []
for i in range(num, 3, -1):
if is_even_square(i): squares.append(i)
return squaresdef find_lower_squares(num):
"""Returns a list of even perfect squares less than a given integer"""
return [i for i in range(num, 3, -1) if is_even_square(i)]import doctest
def find_lower_squares(num):
"""
Returns a list of even perfect squares less than a given integer
>>> find_lower_squares(40)
[36, 16, 4]
"""
return [i for i in range(num, 3, -1) if is_even_square(i)]
doctest.testmod()def generate_square_spiral(num):
"""
Generates a square spiral matrix from a given integer
>>> generate_square_spiral(5)
[[5, 4], [2, 3]]
>>> generate_square_spiral(17)
[[17, 16, 15, 14], [5, 4, 3, 13], [7, 1, 2, 12], [8, 9, 10, 11]]
"""
edge = int(sqrt(num))
square_spiral = [nth_row(num, row) for row in range(edge)]
return square_spiraledge = int(sqrt(num))
return [nth_row(num, row) for row in range(edge)]Context
StackExchange Code Review Q#82771, answer score: 3
Revisions (0)
No revisions yet.