For making your code faster, we can choose more Pythonic ways in some parts of it.
First, let's take a look at convertToList function. The goal of this function is to split digits of a number to a list of int values. I want to follow your algorithm and do this by converting the number to the str and then splitting it.
I want to do this by "List Comprehension":
def convert_to_list(num):
result = [int(x) for x in str(num)]
return result
We made some changes here. First of all, I changed the name of the function from camelCase (convertToList) to snake_case (convert_to_list) because according to the Python style guide, it is the better way. You can read more about the Python style guide at PEP8.
Next change is I replaced all your code with a single line list comprehension. The first advantage, we have written less code. Less code means probably fewer bugs.
But the second advantage here is this code is so much faster. How much? I have written a simple script for it; the result is that on average, the second version is 1.7 times faster.
Now let's move on and take another look at addDigits function. I want to choose pythonic way again here:
def sum_of_digits(number):
num_list = convert_to_list(number)
return sum(num_list)
Like the previous time, I changed the name. I think this name is clearer and everyone could tell what this code does.
For converting the number to a list of digits, I used convert_to_list function instead of writing the whole code again. We are using functions to avoiding duplication, so I think it's a bad idea to write the same code here. For calculating the sum of digits in a list, I strongly recommend you that always use built-in function sum. It's faster, you don't need to write new code and every Python programmer can tell what you are doing at first glance. This code is somehow 1.2 times faster than previous.
Now let's go to the beginning of your code. We want to parse command line parameters and be sure that the input is correct.
Even though we only call those codes once, I think it is a great favor to the readability of the code to move those lines in a separate function.
from re import search
def get_input_from_cmd(args_list):
if len(args_list) != 2:
raise Exception("You should enter a 16-digit number as input argument")
return args_list[1]
def is_input_valid(input_str):
return bool(search(r"\d{16}", input_str))
I separated your code into two functions. The first function gets argv list as an input parameter and if its length is equal to 2, returns the second parameter. Else, it will raise an Exception. There are a lot of people out there who are against exceptions and I agree with most of their reasons. But when we want our program to stop when a bad input came in, I think using exceptions is the best way.
The second function simply uss search function of re module. It checks that the input string only contains 16 digits. If that assumption will be true, True will be returned. Otherwise, the False value is what you get.
Now you can change your main function like this:
def main():
input_string = get_input_from_cmd(sys.argv)
if is_input_valid(input_string):
digits_list = convert_to_list(input_string)
card_checking_sum = card_check(digits_list)
if card_checking_sum % 10 == 0:
print('Valid Card!')
else:
print('Invalid Card!')
else:
print("Invalid Card number")
What we do is if card number is not a 16-digit number, code in the last else will execute. Otherwise, codes in the first if will run. That looks nicer to me.
So now let's go to the last function, the cardCheck.
def card_check(digits_list, count=0):
result = 0
if count % 2 == 0:
digits_list[count] *= 2
if digits_list[count] >= 10:
digits_list[count] = sum_of_digits(digits_list[count])
if count < 15:
card_check(digits_list, count + 1)
else:
return 0
result += sum(digits_list)
return result
There were some problems in your code that I tried to fix. First, you don't need to put if conditions in parentheses. In python, don't need means you should not.
Second, if you run the same code in if and else; you should take that part of code away from if statement. That is what I did with card_check(digits_list, count + 1) line. That line was repeated needlessly.
In the end, for logical operations, you should use operators like and and or, not & and |.
Here is the full code. I hope that helps you:
from re import search
import sys
def get_input_from_cmd(args_list):
if len(args_list) != 2:
raise Exception("You should enter a 16-digit number as input argument")
return args_list[1]
def is_input_valid(input_str):
return bool(search(r"\d{16}", input_str))
def convert_to_list(num):
result = [int(x) for x in str(num)]
return result
def sum_of_digits(number):
num_list = convert_to_list(number)
return sum(num_list)
def card_check(digits_list, count=0):
result = 0
if count % 2 == 0:
digits_list[count] *= 2
if digits_list[count] >= 10:
digits_list[count] = sum_of_digits(digits_list[count])
if count < 15:
card_check(digits_list, count + 1)
else:
return 0
result += sum(digits_list)
return result
def main():
input_string = get_input_from_cmd(sys.argv)
if is_input_valid(input_string):
digits_list = convert_to_list(input_string)
card_checking_sum = card_check(digits_list)
if card_checking_sum % 10 == 0:
print('Valid Card!')
else:
print('Invalid Card!')
else:
print("Invalid Card number")
if __name__ == '__main__':
main()
