5
\$\begingroup\$

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.

\$\endgroup\$
6
\$\begingroup\$

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
    
\$\endgroup\$
6
\$\begingroup\$

I think all that logic you are using to check which digits to multiply by 2 and which not to is a little overengineered. If you generate the digits from right to left, it's always the even positions that get doubled. You can also make your life simpler by generating the digits one at a time, rather than the whole number. If you do it like this, you don't even need to keep the digits in a list, as you can add them to the return number directly. Another possible optimization for the doubled digit result is to use a look-up table for the values. With these ideas in mind, I rewrote your code as:

def luhn(digits):
    """
    Generates a Luhn-algorithm-valid number of `digits` digits.
    """
    digit_sum = 0
    mult = 10
    number = 0
    for j in range(1, digits):
        if j == digits - 1:
            digit = random.randint(1, 9)  # leading digit cannot be zero
        else:
            digit = random.randint(0, 9)
        if j % 2:
            # look-up table computes sum of digits of 2*digit 
            digit_sum += [0, 2, 4, 6, 8, 1, 3, 5, 7, 9][digit]
        else:
            digit_sum += digit
        # build the number, one digit at a time
        number += digit * mult
        mult *= 10
    # Add the check digit
    number += digit_sum * 9 % 10

    return number
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.