HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Reduce square root to simplest radical form

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
radicalsimplestreducerootsquareform

Problem

The function 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 root


Examples:

redsqrt(5.7)
√ 5.7

redsqrt(100)
10

redsqrt(2000)
20 √ 5

redsqrt(2040506)
13 √ 12074

Solution

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.

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 root


Unnecessary 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 max of all factors?



  • You also compute sqrt(n) and max(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 sqrt when 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 error


Lastly, 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 root
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)
>>> sqrt(-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: math domain error
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)

Context

StackExchange Code Review Q#144041, answer score: 5

Revisions (0)

No revisions yet.