patternpythonMinor
Implementation of Luhn Algorithm
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.
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:
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.
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 NoneIt'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] -= 9Basically 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:
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:
-
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:
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:
I would also create a variable
-
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.
- In Python, function names are
snake_case, notPascalCase.
- You should always put spaces around operators, for example
for i in range(0, digits - 1)instead. (That example could be tightened tofor i in range(digits - 1), as ranges implicitly start from 0.)
- I'm not a big fan of the variable names
numornum_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 % 2I 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] % 9Code Snippets
def Luhn(digits):
if digits < 2:
return
# do rest of functionnum = random.randrange(10 ** (digits - 2), 10 ** (digits - 1))
num_digits = [int(digit) for digit in str(num)]range_start = digits % 2range_start = digits % 2
range_stop = math.ceil(digits + range_start - 1) / 2)
for i in range(range_start, range_stop):
# do some stufffor i in range(range_start, range_stop):
idx = 2 * i + digits % 2
num_digits[idx] = 2 * num_digits[idx] % 9Context
StackExchange Code Review Q#97676, answer score: 6
Revisions (0)
No revisions yet.