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

Minimum Value evenly divisible by all integers from 1 to n

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

Problem

I have just started learning python and I am solving Project Euler problems alongside for practice. This is my solution for problem 5. Project Euler asks for the minimum value divisible by all numbers from 1 to 20 but I have made the program so it works from 1 to n.

I think there is a lot of room for improvement in it and I need your suggestions. Please tell me how I can improve it.

```
def least(x):
"""
Variables:
x = Argument of function and the maximum value of range.
n = Number that is going to be tested by loops in each cycle.
a = Number that is going to be added to "n" in each cycle of loop. It is initialized with "x".
For example: if we check for all numbers from 1 to 10, each "n" has to be multiple of 10. So that's why
a will be added to n in each turn.
y = To minimize machine time, I have tried to bring argument of "least" function below 10.
So if x is bigger than 10, y will be x - 10. And then we will do recursion and use "least" on y.

For example: If we check from 1 to 40.
least(40) -> n = least(30) = a (calls least(30) to get n and a)
least(30) -> n = least(20) = a (calls least(20) to get n and a)
least(20) -> n = least(10) = a (calls least(10) to get n and a)
least(10) -> n = 10, a =10, (Brute Check)
(Brute Check)
(At each stage "for loop" checks "n" in each loop cycle and adds "a" until value found.)
"""
if x <= 10:
n = x #The first number to test
a = x #a is number that's going to be added in each turn
else:
y = x - 10
n = least(y) #Recursion to minimize machine load and time
a = n
while True:
d = 0
for i in range(1,x + 1):
if n % i == 0:
d += 1 #counts if i evenly divides.
else:
break #Checks each n

Solution

Your code can be more pythonic.

  • Names of variables and functions should be more meaningful and descriptive. Also - read PEP8.



  • You should encapsulate your code in smaller functions responsible


for one thing. And signal what is the purpose of each function by
it's name (= meaningful names).

  • Use power which python gives you. To save memory instead of range() use xrange() in every case when you only need to iterate over. range() creates a list, so if you do range(1, 99999) it creates a list in memory with 99999 elements. In contrast xrange() is a sequence object that evaluates lazily.



  • There are more efficient algorithms than brute-force. But even your


brute-force approach can be improved. With xranege() you don't
need recursion any more. What is more it's wiser to iterate over
dividers in decreasing order. Which is dead simple in python, just use -1 as third xrange() parameter (step).

Take a look at this example:

def smallest_common_multiple(maximal_divisor):
    for number in xrange(maximal_divisor, sys.maxsize):
        if is_common_multiple(number, maximal_divisor):
            return number

def is_common_multiple(number, maximal_divisor):
    for divisor in xrange(maximal_divisor, 1, -1):
        if number % divisor != 0:
            return False
    return True


Comment:

In smallest_common_multiple() iteration starts from maximal_divisor because smaller numbers doesn't make sense. sys.maxsize works like to infinity.

:-)

Update

As Josay points out this can be even simpler. Thanks Josay!

from itertools import count

def smallest_common_multiple(maximal_divisor):
    for number in count(maximal_divisor):
        if is_common_multiple(number, maximal_divisor):
            return number

def is_common_multiple(number, maximal_divisor):
    return all([number % divisor == 0 for divisor in xrange(maximal_divisor, 1, -1)])

Code Snippets

def smallest_common_multiple(maximal_divisor):
    for number in xrange(maximal_divisor, sys.maxsize):
        if is_common_multiple(number, maximal_divisor):
            return number

def is_common_multiple(number, maximal_divisor):
    for divisor in xrange(maximal_divisor, 1, -1):
        if number % divisor != 0:
            return False
    return True
from itertools import count

def smallest_common_multiple(maximal_divisor):
    for number in count(maximal_divisor):
        if is_common_multiple(number, maximal_divisor):
            return number

def is_common_multiple(number, maximal_divisor):
    return all([number % divisor == 0 for divisor in xrange(maximal_divisor, 1, -1)])

Context

StackExchange Code Review Q#91156, answer score: 4

Revisions (0)

No revisions yet.