patternpythonMinor
Reduce square root to simplest radical form
Viewed 0 times
radicalsimplestreducerootsquareform
Problem
The function
Examples:
redsqrt(n) prints out the most reduced form of the square root of n. It calls on another function perfsq(n), which produces a list of perfect squares less than or equal to n.from math import sqrt
def perfsq(n):
# Returns list of perfect squares less than or equal to n
l = [1]
i, x = 0, 3
while l[-1]+x 1]
if len(l)==0:
print('\u221A',n) # Square root is irreducible
else:
a = int(sqrt(max(l))) # Coefficient
b = int(n/max(l)) # Argument of the square root
print(a,'\u221A',b) # Reduced square rootExamples:
redsqrt(5.7)
√ 5.7
redsqrt(100)
10
redsqrt(2000)
20 √ 5
redsqrt(2040506)
13 √ 12074Solution
Style
Give your code some love and help readers by adding blank lines to separate logical sections of code. It is recommended to use two blank lines before function definition.
Space after commas and around operators also help make the code more readable.
Some of your comments would be better suited as docstrings and some other barely state what you are doing, which we could read from the code already. Comment should focus on why instead.
Lastly using meaningful variable names play a large role in making your algorithm understandable. Avoid abreviations.
Unnecessary work
Instead you could iterate from the nearest integer lower than
A first draft could be:
Reusability
It is bad practice to encore computation and output in the same function as it makes it harder to test and reuse. You should instead
This also means you need better handling of you error case. The standard in Python being to raise an exception. Here
Lastly, returning values let you short circuit your computation and remove the need of the
Give your code some love and help readers by adding blank lines to separate logical sections of code. It is recommended to use two blank lines before function definition.
Space after commas and around operators also help make the code more readable.
Some of your comments would be better suited as docstrings and some other barely state what you are doing, which we could read from the code already. Comment should focus on why instead.
Lastly using meaningful variable names play a large role in making your algorithm understandable. Avoid abreviations.
from math import sqrt
def perfect_square(limit):
accumulation_list = [1]
index, increment = 0, 3
while accumulation_list[-1] + increment 1]
if len(factors) == 0:
print('\u221A', n) # Square root is irreducible
else:
a = int(sqrt(max(factors))) # Coefficient
b = int(n / max(factors)) # Argument of the square root
print(a, '\u221A', b) # Reduced square rootUnnecessary work
- Why compute the perfect squares starting from 1 if you are going to filter this value later on?
- Why compute all these values at all if you're only going to take the
maxof all factors?
- You also compute
sqrt(n)andmax(factor)several time when you could store the result in a variable.
- Most importantly, you compute the perfect squares rather expensively only to recompute its fairly expensive
sqrtwhen you find one as factor.
Instead you could iterate from the nearest integer lower than
sqrt(n) down to 1 and see if its square is a factor of n. Once found you have your factor without computing the remaining values. If none can be found, you know that you have an irreducible square root.A first draft could be:
from math import sqrt
def reduced_sqrt(n):
"""Print most reduced form of square root of n"""
if n < 0:
print('Negative input')
return
root = int(sqrt(n))
found = False
for factor_root in range(root, 1, -1):
factor = factor_root * factor_root
if n % factor == 0:
found = True
reduced = n // factor
if reduced == 1:
# n was a perfect square
print(factor_root)
else:
print(factor_root, '\u221A', reduced)
break
if not found:
# irreducible square root
print('\u221A', n)Reusability
It is bad practice to encore computation and output in the same function as it makes it harder to test and reuse. You should instead
return a couple "coefficient/reduced square root" so that the caller can handle the logic of pretty printing the output.This also means you need better handling of you error case. The standard in Python being to raise an exception. Here
raise ValueError('negative input') should be enough. But you don't even need that as sqrt of a negative number already raise a ValueError:>>> sqrt(-1)
Traceback (most recent call last):
File "", line 1, in
ValueError: math domain errorLastly, returning values let you short circuit your computation and remove the need of the
found flag:from math import sqrt
def reduced_sqrt(n):
"""Return most reduced form of square root
of n as the couple (coefficient, reduced_form)
"""
root = int(sqrt(n))
for factor_root in range(root, 1, -1):
factor = factor_root * factor_root
if n % factor == 0:
reduced = n // factor
return (factor_root, reduced)
return (1, n)
def print_reduced_sqrt(n):
coefficient, reduced = reduced_sqrt(n)
if coefficient == 1:
print('\u221A', reduced)
elif reduced == 1:
print(coefficient)
else:
print(coefficient, '\u221A', reduced)Code Snippets
from math import sqrt
def perfect_square(limit):
accumulation_list = [1]
index, increment = 0, 3
while accumulation_list[-1] + increment <= limit:
accumulation_list.append(accumulation_list[iindex] + increment)
index += 1
increment = 2 * index + 3
return accumulation_list
def reduced_sqrt(n):
"""Print most reduced form of square root of n"""
if n < 0:
print('Negative input')
return
if sqrt(n).is_integer():
print(int(sqrt(n)))
return
# Find perfect squares that are factors of n
factors = [square for square in perfect_square(n/2) if n % square == 0 and square > 1]
if len(factors) == 0:
print('\u221A', n) # Square root is irreducible
else:
a = int(sqrt(max(factors))) # Coefficient
b = int(n / max(factors)) # Argument of the square root
print(a, '\u221A', b) # Reduced square rootfrom math import sqrt
def reduced_sqrt(n):
"""Print most reduced form of square root of n"""
if n < 0:
print('Negative input')
return
root = int(sqrt(n))
found = False
for factor_root in range(root, 1, -1):
factor = factor_root * factor_root
if n % factor == 0:
found = True
reduced = n // factor
if reduced == 1:
# n was a perfect square
print(factor_root)
else:
print(factor_root, '\u221A', reduced)
break
if not found:
# irreducible square root
print('\u221A', n)>>> sqrt(-1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: math domain errorfrom math import sqrt
def reduced_sqrt(n):
"""Return most reduced form of square root
of n as the couple (coefficient, reduced_form)
"""
root = int(sqrt(n))
for factor_root in range(root, 1, -1):
factor = factor_root * factor_root
if n % factor == 0:
reduced = n // factor
return (factor_root, reduced)
return (1, n)
def print_reduced_sqrt(n):
coefficient, reduced = reduced_sqrt(n)
if coefficient == 1:
print('\u221A', reduced)
elif reduced == 1:
print(coefficient)
else:
print(coefficient, '\u221A', reduced)Context
StackExchange Code Review Q#144041, answer score: 5
Revisions (0)
No revisions yet.