Krdytkyu

I've implemented Luhn's algorithm for checking credit card numbers.
My code works, but I wanted to learn about a more efficient and more Pythonic way of doing this.

def validate_credit_card_number(card_number):

    #start writing your code here



    #Step 1a - complete

    temp_list=list(str(card_number))

    my_list=

    list1 = temp_list[-2::-2]

    list2=temp_list[::-2]

    list2 = [int (n) for n in list2]

    #print(list2)

    my_list=[int(n) for n in list1]

    #print(my_list)

    list1 = [int(n)*2 for n in list1]

    t_list=list1



    for el in list1:

        sum_res=0



        if el>9:

            idx = list1.index(el)

            t_list.pop(idx)



            while el:

                rem = el%10

                sum_res+=rem

                el = el//10 

            t_list.insert(idx, sum_res)

    #print(t_list)



    #step 1b



    list1_sum=sum(t_list)

    list2_sum = sum(list2)

    #print(b_list)

    final_sum = list1_sum+ list2_sum

    #print(final_sum)



    if final_sum%10==0:

        return True

    return False



card_number= 1456734512345698 #4539869650133101  #1456734512345698 # #5239512608615007



result=validate_credit_card_number(card_number)

if(result):

    print("credit card number is valid")

else:

    print("credit card number is invalid")

Style

Python has a style guide called PEP 8. It is a good habit to try to follow it.

In your case, that would mean fixing the missing whitespaces, removing parenthesis in if(result).

Also, you could get rid of old commented code and add a proper docstring if you want to describe the behavior of the function.

Tests

It could be a good idea to write tests before trying to improve your code.

I wrote a very simple code but you could get this chance to dive into unit testing frameworks:

TESTS = [

    (1456734512345698, False),

    (4539869650133101, True),

    (1456734512345698, False),

    (5239512608615007, True),

]



for (card_number, expected_valid) in TESTS:

    valid = validate_credit_card_number(card_number)

    assert valid == expected_valid

Useless test

At the end of the function, you can return directly:

return final_sum % 10 == 0

Useless variables

The my_list variable is not required.

Also, we can take this chance to get rid of the variables at the end of the function:

return (sum(t_list) + sum(list2)) % 10 == 0

Conversion of card_number

At the beginning of the function, you could convert the parts of card_number directly to integer so that you do it in a single place.
Also, that removed the need to the call of list. We just have:

temp_list = [int(c) for c in str(card_number)]

And we can get rid of the line:

list2 = [int (n) for n in list2]

At this stage, the code looks like:

def validate_credit_card_number(card_number):

    temp_list = [int(c) for c in str(card_number)]



    list1 = temp_list[-2::-2]

    list1 = [2 * n for n in list1]



    list2 = temp_list[::-2]



    t_list = list1



    for el in list1:

        sum_res = 0



        if el > 9:

            idx = list1.index(el)

            t_list.pop(idx)



            while el:

                rem = el % 10

                sum_res += rem

                el = el // 10

            t_list.insert(idx, sum_res)



    return (sum(t_list) + sum(list2)) % 10 == 0

Yet another useless variable

t_list aliases list1 (I am not sure if this is intended if if you were planning to have a copy of list1). Whenever you update the list through one variable, the other is affected as well. I highly recommend Ned Batchelder's talk about names and values.

In your case, we can get rid of t_list completely without changing the behavior of the function.

Simplify list logic

You go through multiple steps to modify list1 (or t_list) : index, pop, index. These steps are more expensive/complicated than required. At the end of the day, you do not care about list1, you just want its final sum. You could keep track of the sum directly:

sum1 = 0

for el in list1:

    if el > 9:

        sum_res = 0

        while el:

            rem = el % 10

            sum_res += rem

            el = el // 10

        sum1 += sum_res

    else:

        sum1 += el



return (sum1 + sum(list2)) % 10 == 0

We can take this chance to perform the multiplication in the loop to remove a list comprehension.

Also, we can initialise the sum with sum(list2) so that we don't have to add them at the end:

def validate_credit_card_number(card_number):

    temp_list = [int(c) for c in str(card_number)]



    list1 = temp_list[-2::-2]

    list2 = temp_list[::-2]



    total_sum = sum(list2)

    for el in list1:

        el *= 2

        if el > 9:

            sum_res = 0

            while el:

                rem = el % 10

                sum_res += rem

                el = el // 10

            total_sum += sum_res

        else:

            total_sum += el



    return total_sum % 10 == 0

Math logic

The code uses 10 (the base used for computations) everywhere except for one 9 which seems unexpected. You could write: el >= 10 instead.

Also, that check is not required because the logic applies exactly the same way for elements smaller than 10:

for el in list1:

    el *= 2

    sum_res = 0

    while el:

        rem = el % 10

        sum_res += rem

        el = el // 10

    total_sum += sum_res

Also, you could use el //= 10 but you can get the best ouf of the Python builtins by using divmod returning both the quotient and the remainder:

    while el:

        el, rem = divmod(el, 10)

        sum_res += rem

    total_sum += sum_res

Then, it becomes clear that the variable sum_res is not really required as we could use total_sum instead:

    while el:

        el, rem = divmod(el, 10)

        total_sum += rem

"Final" code

def validate_credit_card_number(card_number):

    temp_list = [int(c) for c in str(card_number)]



    list1 = temp_list[-2::-2]

    list2 = temp_list[::-2]



    total_sum = sum(list2)

    for el in list1:

        el *= 2

        while el:

            el, rem = divmod(el, 10)

            total_sum += rem



    return total_sum % 10 == 0



TESTS = [

    (1456734512345698, False),

    (4539869650133101, True),

    (1456734512345698, False),

    (5239512608615007, True),

]



for (card_number, expected_valid) in TESTS:

    valid = validate_credit_card_number(card_number)

    assert valid == expected_valid

More simplification

Thinking about it, things can still be simplified a lot.
What you are doing with the while loop can be performed using str conversion:

total_sum = sum(list2)

for el in list1:

    total_sum += sum(int(c) for c in str(2 * el))

Going further (too far?), this leads to:

def validate_credit_card_number(card_number):

    temp_list = [int(c) for c in str(card_number)]



    list1 = temp_list[-2::-2]

    list2 = temp_list[::-2]



    total_sum = sum(list2) + sum(sum(int(c) for c in str(2 * el)) for el in list1)

    return total_sum % 10 == 0

I assume that card_number is an integer. As such, you should not convert it to a str, nor convert it to a list. You should be doing integer operations only, not list operations.

For your learning purposes, I won't rewrite the algorithm for you, but try to rewrite it such that it's represented as a loop over the credit card number, which successively gets divided by 10.