0

I'm using the Luhn algorithm to find ids check number. for Example - input - 12345678 , output - 2. I'm trying to do it in the most efficient way possible, as well as in the fewest lines possible. So far I wrote this :

ID = input("Enter your first 8 digits of your id : ")
print(10 - sum(map(lambda a: a if a <= 9 else int(str(a)[0]) + int(str(a)[1]), [int(ID[i]) if i % 2 
== 0 else int(ID[i]) * 2 for i in range(8)])) % 10) # if digit is 0 prints 10

I'm not satisfied with the int string conversions. But I'm Looking for any improvement.

khelwood
  • 55,782
  • 14
  • 81
  • 108

1 Answers1

1

and welcome to StackOverflow!

Super short, but with a dependency

If you can rely on external dependencies, take a look at the luhn package. You can install it with pip install luhn, and then your program becomes:

from luhn import generate

ID = input("Enter your first 8 digits of your id : ")
print(generate(ID))

Can't get much cleaner than this.

Changing your code

But let's suppose you don't want to use a third-party package. Let us first put your code in a function and format it:

def luhn(number):
    return (
        10 - sum(
            map(
                lambda a: a if a <= 9 else int(str(a)[0]) + int(str(a)[1]),
                [
                    int(number[i])
                    if i % 2 == 0
                    else int(number[i]) * 2
                    for i in range(8)
                ]
            )
        ) % 10
    )

Step 1: list to generator

You can simply replace your list comprehension with a generator expression to make it a bit faster since you don't need to build the whole list first:

def luhn(number):
    return (
        10 - sum(
            map(
                lambda a: a if a <= 9 else int(str(a)[0]) + int(str(a)[1]),
                ( # <--- changed this
                    int(number[i])
                    if i % 2 == 0
                    else int(number[i]) * 2
                    for i in range(8)
                ) # <--- changed this
            )
        ) % 10
    )

Step 2: Get rid of map

I personally don't like using map when you have to write a lambda as well. It just kills the two purposes of map: simpler code and fast, C-speed execution.

For instance, map(int, myseq) is great, extremely fast and clean. But map(lambda x: int(x), myseq) is cumbersome (you have to give a name x to myseq items) and slow (because lambdas don't benefit from the C-like speed of builtin functions). In this case, I would say that int(x) for x in myseq reads much nicer.

Applying that to the previous function:

def luhn(number):
    return (
        10 - sum(
            a if a <= 9 else int(str(a)[0]) + int(str(a)[1])
            for a in (
                int(number[i])
                if i % 2 == 0
                else int(number[i]) * 2
                for i in range(8)
            ) # bonus: we have one less indentation level!
        ) % 10
    )

Step 3: Get rid of one if-else

In the second for loop, we have x if i % 2 == 0 else x * 2 where x = int(number[i]). But since i % 2 is either 0 or 1, you can benefit from this and write x * (1 + i%2). So our function becomes:

def luhn(number):
    return (
        10 - sum(
            a if a <= 9 else int(str(a)[0]) + int(str(a)[1])
            for a in (int(number[i]) * (1 + i%2) for i in range(8))
        ) % 10
    )

Getting prettier, right?

Step 4: Split "even" and "odd" characters

Now think with me. When will i % 2 be equal to 0 and when will it be 1? It is 0 when i is even, and 1 when i is odd. So, in the expression int(number[i]) * (1 + i%2), we will have int(number[i]) when i is even, and int(number[i]) * 2 when i is odd. Supposing that number is 314159, then the "even characters" (characters in even positions) 345 are returned as-is, and the "odd characters" 119 are multiplied by 2.

Moreover, the expression a if a <= 9 else int(str(a)[0]) + int(str(a)[1]) is also trivial for even characters: since they are returned as-is, they obviously satisfy a <= 9, and then they are also returned as-is.

Noting that even and odd characters can be obtained by slicing:

s = 314159
even = s[::2] # 345
odd = s[1::2] # 119

So we can split our function into two parts. It gets bigger now, but afterwards it'll get better:

def luhn(number):
    evens = number[::2]
    even_sum = sum(map(int, evens)) # sum even characters

    odds = number[1::2]
    odd_sum = sum(
        a if a <= 9 else int(str(a)[0]) + int(str(a)[1])
        for a in (int(odd) * 2 for odd in odds)
    ) # sum odd characters

    return (
        10 - (even_sum + odd_sum) % 10
    )

Step 5: More refactoring

In "a if a <= 9 else int(str(a)[0]) + int(str(a)[1])", what this piece of code is saying is: sum the digits of a. Because, if a has only one digit (a <= 9) we are just returning it; otherwise, we are summing its first and second digits.

But then, taking a closer look, we are summing this sum of digits for all numbers yielded by str(int(odd) * 2) for odd in odds! This means that we can first concatenate all the digits, and then sum them all:

def luhn(number):
    evens = number[::2]
    even_sum = sum(map(int, evens)) # sums even characters

    odds = number[1::2]
    multiplied_by_two = (str(int(odd) * 2) for odd in odds)
    all_digits = ''.join(multiplied_by_two)
    odd_sum = sum(map(int, all_digits))

    return (
        10 - (even_sum + odd_sum) % 10
    )

And we finally merge the two sums into one:

def luhn(number):
    evens = number[::2]

    odds = number[1::2]
    multiplied_by_two = (str(int(odd) * 2) for odd in odds)
    all_digits = ''.join(multiplied_by_two)

    total_sum = sum(map(int, evens + all_digits))

    return (
        10 - total_sum % 10
    )

Now we just collapse into a single expression:

def luhn(number):
    return (
        10 - sum(
            map(
                int,
                number[::2] + ''.join(
                    str(int(odd) * 2) for odd in number[1::2]
                )
            )
        ) % 10
    )

Finally, we make it a one-liner and finish your program:

ID = input("Enter your first 8 digits of your id : ")
print(10 - sum(map(int, ID[::2] + ''.join(str(int(odd) * 2) for odd in ID[1::2]))) % 10)

Some metrics

By comparing our solutions:

import timeit

ID = '12345678'
print(timeit.timeit(lambda: 10 - sum(map(lambda a: a if a <= 9 else int(str(a)[0]) + int(str(a)[1]), [int(ID[i]) if i % 2
== 0 else int(ID[i]) * 2 for i in range(8)])) % 10, number=1000000))
print(timeit.timeit(lambda: 10 - sum(map(int, ID[::2] + ''.join(str(int(odd) * 2) for odd in ID[1::2]))) % 10, number=1000000))

you can see that the latter is ~30% faster.

ruancomelli
  • 610
  • 8
  • 17