0

I created a function to compute HCF of 2 numbers.When I used my parameters as 24 and 36,I am getting the correct answer but when I used the different numbers,I am getting an error : ValueError: max() arg is an empty sequence Here is the code:

def computeHCF(num1,num2):
lst1 = []
lst2 = []
for i in range(2,num1+1):
    if i > 1:
        if num1 % i == 0:
            lst1.append(i)
for i in range(2,num2+1):
    if i > 1:
        if num2 % i == 0:
            lst2.append(i)

HCF = []
for e1,e2 in zip(lst1,lst2):
    if e1 == e2:
         HCF.append(e1)
HCF = max(HCF)
print('The HCF is :',HCF)

I know there are other solutions to this but I tried implementing this method. Edit : Ignore the indentation here.I know where the logic is incorrect.

Sibtain Reza
  • 513
  • 2
  • 14
  • why on earth would you implement this in this manner this is probably the absolute least performant HCF (also known as GCD) algorithm I have seen ... just curious why you would choose this method? – Joran Beasley Feb 08 '20 at 05:16
  • Please share the entire error message, as well as a [mcve]. What do/don’t you understand from that error – AMC Feb 08 '20 at 23:59
  • I was wondering the same thing as @JoranBeasley, with a key difference: Why such a seemingly unintuitive and convoluted solution? When I think GCD, the first, most naive method which comes to mind is to build two sets, each containing the divisors of one of the numbers, and then returning the maximum of the intersection of the two sets. That’s 3 lines of code. – AMC Feb 09 '20 at 00:05

1 Answers1

0

Zip iterates over both lists simultaneously, so it will only find an HCF if factors are at the same position in lst1 and lst2. Instead, you just want to find the common elements in the two lists. Try the following:

HCF = []
for e1 in lst1:
    if e1 in lst2:
         HCF.append(e1)

Alternatively, from here:

HCF = list(set(lst1).intersection(lst2))

This works with 24 and 36 because their factors line up. I guess you were getting 4 as the answer?

24: 2, 3, 4, 6, 8, 12
36: 2, 3, 4, 9, 12, 18
Dominic D
  • 1,778
  • 2
  • 5
  • 12