2
\$\begingroup\$
list = int(input("Enter the first 15 digits of your credit card number without spaces and dashes: "))
checkDigit = int(input("Enter the last digit of the credit card number: "))
numbers = []
for elements in str(list):
    numbers.append(int(elements))

def main():
    n = None
    total_odd = 0
    total_even = 0
    count = 0
    for element in numbers[::2]:
        total_odd += element
        if element > 4:
            count += 1
    for element in numbers[1::2]:
        total_even += element
    total = total_even + (total_odd * 2) + count
    if total % 10 != 0:
        for i in range(1,10):
            if (total + i) % 10 == 0:
                n = i
    else:
        n = 0
    validation(n)

def validation(n):
    if n == checkDigit:
        print("Your credit card is valid!")
    else:
        print("Your credit card in not valid!")

main()
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Please add a description of where this problem came from and a link if applicable. \$\endgroup\$
    – Reinderien
    Jun 22 at 17:56

1 Answer 1

3
\$\begingroup\$

Your program and functions are structured ineffectively. You have some code at the top level (collecting user input and converting it to numbers), then most of the card-checking logic in main(), and a tiny bit of printing logic in validation(). Much better would be a structure like the sketch below. In main() we handle the user interactions (getting input and printing) and we use a function like validate_credit_card_number() to handle the detailed logic and return true or false. This structure is better for a variety of reasons, one being that it makes it much easier to subject the validation function (which is where the greatest potential for errors lies) to a battery of automated tests. Note also that we end up with no logic at the top level (the sooner you embrace this habit, the better).

def main():
    ccnum = input(...)
    if validate_credit_card_number(ccnum):
        print('valid')
    else:
        print('invalid')

def validate_credit_card_number(ccnum):
    # Details shown below.
    ...

if __name__ == '__main__':
    main()

Converting the card number to the needed lists of digits. There are easier ways to handle such tasks, mostly using indexing and slicing techniques that you already appear to know about:

# Convert a big int to a list of its digits.
digits = [int(d) for d in str(ccnum)]

# Grab the last digit.
check_digit = digits[-1]

# Grab two groups of every-other digits, taken from the first 15 digits.
odds = digits[0:16:2]
evens = digits[1:16:2]

Counting the number of odds greater than 4. This can also be done more simply via Python's sum() function and the fact that boolean outcomes can be treated arithmetically.

n_big_odds = sum(o > 4 for o in odds)

Simplifying the computation of the expected check digit. Your approach seems overly complex. Via experimentation with different inputs, I inferred the following pattern. [Someone more fluent in the rules of modulo arithmetic than I might be able to simplify it further.]

checksum = sum(evens) + (sum(odds) * 2) + n_big_odds
expected = (10 - (checksum % 10)) % 10

The final check. And then the function should just return a boolean value.

return expected == check_digit
\$\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.