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

Implementation of Luhn Algorithm

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

Problem

I've implemented the well known Luhn Algorithm in Python. It's a simple one, so it's good for beginners.

import random
from math import ceil

def Luhn(digits): 
    if digits >= 2:
        num = random.randrange(10**(digits-2),10**(digits-1))
        num_digits = list(str(num))

        for i in range(0,digits-1):
            num_digits[i] = int(num_digits[i])

        if digits % 2 == 0:
            range_start = 0
        else:
            range_start = 1 

        for i in range(range_start,ceil((digits+range_start-1)/2)):
            if digits % 2 == 0:
                num_digits[2*i] *= 2
                if num_digits[2*i] > 9:
                    num_digits[2*i] -= 9
            else:
                num_digits[2*i-1] *= 2
                if num_digits[2*i-1] > 9:
                    num_digits[2*i-1] -= 9

        checksum = sum(num_digits)
        last_digit = checksum % 10

        if last_digit != 0:
            checknum = 10 - last_digit
        else:
            checknum = 0

        num = num*10+checknum
        return num
    else:
        return None


It's a function that takes 1 parameter (digits) and returns a valid number with a given number of digits.
The code is pretty straightforward, except for this part:

if digits % 2 == 0:
    range_start = 0
else:
    range_start = 1 

for i in range(range_start,ceil((digits+range_start-1)/2)):
    if digits % 2 == 0:
        num_digits[2*i] *= 2
        if num_digits[2*i] > 9:
            num_digits[2*i] -= 9
    else:
        num_digits[2*i-1] *= 2
        if num_digits[2*i-1] > 9:
            num_digits[2*i-1] -= 9


Basically what it does is the 'multiplication by 2' part of the algorithm. This part was intentional, so don't take this in consideration. I just wanted to challenge myself.

I would like to get some feedback about the code and things that can be changed.

Solution

Some style notes:

  • In Python, function names are snake_case, not PascalCase.



  • You should always put spaces around operators, for example for i in range(0, digits - 1) instead. (That example could be tightened to for i in range(digits - 1), as ranges implicitly start from 0.)



  • I'm not a big fan of the variable names num or num_digits. At the very least spell out the word "number" – characters are cheap.



Some more substantial suggestions:

-
There should be a docstring on your function. Right now, it's not clear what it's doing, or how I should use it. (I think it creates a number which complies to the Luhn formula, but I'm guessing.) Also, there are no comments. You should make it clear why you've written this code – explain how it relates back to Luhn's algorithm.

-
To save yourself an indentation level, I'd turn the if statement into an early return:

def Luhn(digits):
    if digits < 2:
        return
    # do rest of function


-
At the start of the function, you create a large N-digit number, convert to a string, then list, and then turn each list element into an integer. You can simplify this slightly:

num = random.randrange(10 ** (digits - 2), 10 ** (digits - 1))
num_digits = [int(digit) for digit in str(num)]


Note that this implementation precludes the returned number starting with 0. I don't know if that was intentional, but if it was, you should have a comment explaining why.

(Luhn's algorithm is generally used with identification numbers, not just base-10 integers, so it's quite plausible it could encounter a number whose first digit is 0.)

-
You can tidy up the if statement as follows:

range_start = digits % 2


I would also create a variable range_stop, which makes the level of abstraction in the range limits more consistent:

range_start = digits % 2
range_stop = math.ceil(digits + range_start - 1) / 2)

for i in range(range_start, range_stop):
    # do some stuff


-
Within the for loop, you have very similar repeated code. I'd pull out the index into a separate variable, and then you can cut down on repetition.

for i in range(range_start, range_stop):
    idx = 2 * i + digits % 2
    num_digits[idx] = 2 * num_digits[idx] % 9

Code Snippets

def Luhn(digits):
    if digits < 2:
        return
    # do rest of function
num = random.randrange(10 ** (digits - 2), 10 ** (digits - 1))
num_digits = [int(digit) for digit in str(num)]
range_start = digits % 2
range_start = digits % 2
range_stop = math.ceil(digits + range_start - 1) / 2)

for i in range(range_start, range_stop):
    # do some stuff
for i in range(range_start, range_stop):
    idx = 2 * i + digits % 2
    num_digits[idx] = 2 * num_digits[idx] % 9

Context

StackExchange Code Review Q#97676, answer score: 6

Revisions (0)

No revisions yet.