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.

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jul 22, 2015 at 10:45
\$\endgroup\$
2

2 Answers 2

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
    
answered Jul 22, 2015 at 13:03
\$\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
answered Jul 22, 2015 at 14:03
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.