Factoring Polynomials Completely

I am relativly new to Python and I decided to try to write code that would factor any polynomial using the Rational Root Theorem and synthetic division. I have two questions.

• Is there any way I can clean up my code or any common practices that I should know about?

• How could I allow my function to factor polynomials with two or more factors that are not binomials? If this is not possible without significant modification to my code, then just let me know without going into the details.

My program takes a user input for the coefficients of the polynomial. If the polynomial has no term of the degree requested, type '0'. The output is a string that contains the factors in a format like:


\$3(2x - 1)(2x + 1)(9x^2 + 4x + 4)\$

My code is below:

```
from fractions import Fraction
def gcf(numbers):
gcf = 1
abs_numbers = [abs(i) for i in numbers]
large = max(abs_numbers)
if numbers[0] >= 0:
for i in range(large, 1, -1):
if all(number % i == 0 for number in abs_numbers) == True:
gcf = i
break
else:
for i in range(-large, -1):
if all(number % i == 0 for number in abs_numbers) == True:
gcf = i
break
return gcf
def factors(x):
factors = []
for i in range(1, abs(int(x)) + 1):
if x % i == 0:
factors.append(i)
return factors
def remove_duplicates(input):
output = []
for x in input:
if x not in output:
output.append(x)
return output
def fractions(lista, listb):
fractions = []
lista = [float(i) for i in lista]
listb = [float(i) for i in listb]
for i in lista:
for j in listb:
fractions.extend([i / j, -i / j])
fractions = remove_duplicates(fractions)
return(fractions)
def synthetic_division(coefficients, divisor):
results = [float(coefficients[0])]
for i in range(len(coefficients) - 1):
results.append(results[-1] * divisor + coe

Coding style

There's a standard for coding style in Python called PEP8.
I suggest to go through it and follow it.

Don't use a variable if you don't need one

In the gcf function, there is a variable called gcf,
the same as the function name,
which can be confusing.
Luckily there's no need for it, you can easily remove all references to it:

def gcf(numbers):
    abs_numbers = [abs(i) for i in numbers]
    large = max(abs_numbers)
    if numbers[0] >= 0:
        for i in range(large, 1, -1):
            if all(number % i == 0 for number in abs_numbers) == True:
                return i
    else:
        for i in range(-large, -1):
            if all(number % i == 0 for number in abs_numbers) == True:
                return i
    return 1

Avoid duplicated logic

Notice that the loops in both branches of the if-else are the same except the range. You can reduce code duplication by first putting the range into a variable, and then perform the loop:

def gcf(numbers):
    abs_numbers = [abs(i) for i in numbers]
    large = max(abs_numbers)
    if numbers[0] >= 0:
        num_range = range(large, 1, -1)
    else:
        num_range = range(-large, -1)

    for i in num_range:
        if all(number % i == 0 for number in abs_numbers) == True:
            return i
    return 1

Use boolean expressions directly

Instead of:

if all(number % i == 0 for number in abs_numbers) == True:

You can use a boolean expression directly, without == True:

if all(number % i == 0 for number in abs_numbers):

Use list comprehensions

This function can be rewritten in a more compact way using list comprehensions:

def factors(x):
    factors = []
    for i in range(1, abs(int(x)) + 1):
        if x % i == 0:
            factors.append(i)
    return factors

Like this:

def factors(x):
    return [i for i in range(1, abs(int(x)) + 1) if x % i == 0]

Odd breaks

A few odd things in this bit of code:

for i in possible_roots:
    results = synthetic_division(coefficients, i)
    if results[-1] == 0:
        results.pop(-1)
        return([-i, results])
        break
else:
    return False

The break after a return is pointless.
And if there is no other statement in a function after the for ... else,
then you can rewrite without the else, which is simpler:

for i in possible_roots:
    results = synthetic_division(coefficients, i)
    if results[-1] == 0:
        results.pop(-1)
        return([-i, results])
return False

But, another thing that's really not good here is that the function returns two kinds of values: it can be list or it can be boolean.
This is going to be confusing and hard to use for callers.
I suggest to rethink the design,
make all functions return one kind of value, not more.

StackExchange Code Review Q#105554, answer score: 6