0
def getLCM(a, b):
    c, d = max(a, b), min(a, b)
    while c != d:
        temp = c - d
        c, d = max(temp, d), min(temp, d)

    return a * b // c


def nlcm(num):
    temp = 1
    while len(num) != 0:
        temp = getLCM(temp, num[-1])
        num.pop()
    return temp

print(nlcm([2,6,8,14,5]));

I need to "quickly" get answer this problem. in test case, my code is very slow.

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
전현근
  • 21
  • 2
  • 4
    Couldn't you find something else to do in that 0.023 seconds it took to run that program? Maybe grab a coffee or play with the kids :-) – paxdiablo Oct 27 '17 at 01:20
  • 1
    i am getting an answer instantaneously on my machine.. are you timing in milliseconds? – 0TTT0 Oct 27 '17 at 01:21
  • Maybe number of num is large, my code is slow. So I need to get fast my code – 전현근 Oct 27 '17 at 02:14

2 Answers2

2

There are existing gcd implementations in Python, and LCM is definable in terms of gcd, so you may as well avoid reinventing the wheel. Specifically:

gcd(a, b) x lcm(a, b) = a x b

With Python 3.5 and higher, there is an accelerated gcd function on the math module, so lcm can simplify to:

from math import gcd

def getLCM(a, b):
    return a * b // gcd(a, b)

On older Python, it's not accelerated, but fractions.gcd is a decent implementation provided for you, so you can use it instead, or to use the best possible gcd on whatever version you run on, a nested import attempt works:

try:
    from math import gcd
except ImportError:
    from fractions import gcd

Your nlcm loop could also be simplified: You don't need to destructively iterate manually, just loop:

def nlcm(num):
    temp = 1
    for x in num:
        temp = getLCM(temp, x)
    return temp

Or if you want to get clever, use reduce (functools.reduce on Python 3.x) since it does exactly what that simplified loop is already doing:

from functools import reduce

def nlcm(nums):
    return reduce(getLCM, nums, 1)
ShadowRanger
  • 143,180
  • 12
  • 188
  • 271
  • Thank you for your answer. thanks to you, I know about the 'reduce'. But in my study class, I can't use function 'gcd'. – 전현근 Oct 27 '17 at 01:56
  • @전현근: Well, you can still use `fractions.gcd` as a better implementation, since [the code for it is pure Python](https://github.com/python/cpython/blob/3.4/Lib/fractions.py#L17) and can literally be copied; just make sure you understand it. ;-) – ShadowRanger Oct 27 '17 at 01:58
0

Assuming the "long execution" is not in the example you provided but rather with bigger inputs, you can add memoization and increate the time getLCM() is calculated in case it's called again with the same numbers:

hash = dict() # we'll save here the calculated results

def getLCM(a, b):
    global hash
    if (a, b) in hash:  # if it was already calculated
        return hash[(a, b)]  # use the cached result
    c, d = max(a, b), min(a, b)
    while c != d:
        temp = c - d
        c, d = max(temp, d), min(temp, d)

    hash[(a, b)] = a * b // c. # cash the result
    return a * b // c
Natalie
  • 173
  • 1
  • 3
  • Note: If you're on Python 3.2 or higher, [`functools.lru_cache`](https://docs.python.org/3/library/functools.html#functools.lru_cache) can give you this caching behavior as a decorator on the original (uncached) function, avoiding the ugliness of mixing the caching logic with the computational logic. – ShadowRanger Oct 27 '17 at 01:27
  • True, though you have to take into account that this caching is... LRU meaning, remembers only the most recently numbers. And since it's limited in size - you should ensure that it fits your application behavior. – Natalie Oct 27 '17 at 02:01
  • The name is slightly inaccurate. The *default* is LRU with a 128 entry bound, but it can be made infinite trivially (you just pass `None` as the `maxsize`). It runs faster that way too (no overhead dealing with cache eviction). – ShadowRanger Oct 27 '17 at 02:06
  • @ShadowRanger thanks I wasn't aware of that! – Natalie Nov 02 '17 at 17:36