# How to find the common factors of 2 numbers [closed]

Print the number of common factors of a and b.

input > 10, 15

Output > 2

The common factors of 10, 15 are 1 and 5

My code

def print_factors(x,y):
l = []
for i in range(1, x + 1):
if x % i == 0:
l.append(i)
m = []
for i in range(1, y + 1):
if y % i == 0:
m.append(i)
print (list(set(l).intersection(m)))
#len(print (list(set(l).intersection(m))))

num1 = int(input("Enter a number: "))
num2 = int(input("Enter a number: "))

print_factors(num1,num2)


Is there any better way to optimize, like list comprehension. or using zip module

• What is the intended output? The number 2 or the list [1, 5]? – Martin R Aug 16 '19 at 13:12
• @sim if you accept some answer, I guess you might want to upvote that answer as well using the ^ sign. – user203258 Aug 16 '19 at 18:41
• I am voting to close as off-topic because the code and specification do not agree, and so the code cannot be considered working. – VisualMelon Aug 16 '19 at 22:27

# Indentation & style

One of the most distinctive features of Python is its use of indentation to mark blocks of code. Your code is not properly indented (each level of indentation should be equivalent to 4 spaces not like this:

def print_factors(x,y):
l = []


like this:

def print_factors(x,y):
l = []


Check pep8 https://www.python.org/dev/peps/pep-0008/ the official Python style guide, you'll find it helpful when learning the basics of Python code style and naming conventions.

# Return

a function should not print anything, it should return

print (list(set(l).intersection(m)))
#len(print (list(set(l).intersection(m))))


and print is a function in Python 3.x, so it's very obvious it has no length len(the commented line).

# len

len() is used to get the length of sequences:

my_list = [1, 2, 3, 4]
print(len(my_list))


output: 4

# Docstrings

Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. An object's docsting is defined by including a string constant as the first statement in the object's definition. it is generally a good practice to use docstrings in your functions explaining what the function does that are usually enclosed within triple quotes:

def print_factors(number1, number2):
"""Return factors of number1 and number2."""
# do things
# return something


# Descriptive variable names

l = []
for i in range(1, x + 1):
if x % i == 0:
l.append(i)
m = []
for i in range(1, y + 1):
if y % i == 0:
m.append(i)
print (list(set(l).intersection(m)))


what is l, what is i what is x ... names should be descriptive to improve the readability of your code (whether people that are reading your code and trying to understand or you reading your own code in the future) and here's an example:

factors_1 = []
factors_2 = []
for divisor in range(1, number1):
if number1 % divisor == 0:
factors1.append(divisor)
for divisor in range(1, number2):
if number2 % divisor == 0:
factors2.append(divisor)


# The code

1. Your function is not even doing what it should be doing (returning the number of common factors not the factors themselves.
2. You don't have to iterate over the whole range of the number, you iterate up to the square root of the number is sufficient to find all factors.

This is how the code might look like:

def get_factors(number):
"""Generate factors of number."""
yield 1
for divisor in range(2, int(number ** 0.5) + 1):
if number % divisor == 0:
if number // divisor != divisor:
yield divisor
yield number // divisor
if number // divisor == divisor:
yield divisor

if __name__ == '__main__':
number1 = int(input('Enter first number: '))
number2 = int(input('Enter second number: '))
number_1_factors = set(get_factors(number1))
print(f'Factors of {number1}: {number_1_factors}')
number_2_factors = set(get_factors(number2))
print(f'Factors of {number2}: {number_2_factors}')
common_factors = number_1_factors & number_2_factors
print(f'Number of common factors of {number1} and {number2}: {len(common_factors)} factors.')