Credit card checking

I am practicing test-driven development in Python. Feedback about both the tests and the code is appreciated. I'm using luhn algorithm and regex to check credit card numbers for validity.

Here are the tests:

from unittest import TestCase
import luhn_check

class TestLuhnCheck(TestCase):

    def setUp(self):
        pass
    def test_known_sum(self):
        self.assertEquals(67, luhn_check.check_sum("7992739871"))

    def test_check_digit(self):
        self.assertEquals(3, luhn_check.find_check_digit("7992739871"))

    def test_check_valid(self):
        self.assertFalse(luhn_check.is_luhn_valid("abcdefg"))
        self.assertTrue(luhn_check.is_luhn_valid("79927398713"))

    def test_check_invalid(self):
        self.assertFalse(luhn_check.is_luhn_valid("79927398714"))

    def test_regex(self):
        self.assertTrue(luhn_check.check_regex("346340033233241"))
        self.assertFalse(luhn_check.check_regex("996340033233241"))

    def test_check_cc(self):
        #amex
        self.assertTrue(luhn_check.is_valid_cc("346340033233241"))
        #discover
        self.assertTrue(luhn_check.is_valid_cc("6011066253550425"))
        #visa
        self.assertTrue(luhn_check.is_valid_cc("4485332611430235"))
        #mc
        self.assertTrue(luhn_check.is_valid_cc("5463393720504875"))
        #bad:
        self.assertFalse(luhn_check.is_valid_cc("abcdefg"))
        self.assertFalse(luhn_check.is_valid_cc("11abcdefg"))
        self.assertFalse(luhn_check.is_valid_cc("946340033233241"))
        self.assertFalse(luhn_check.is_valid_cc("546339372050487500"))

and here is the module:

```
import re

def sum_digits(numeric_string):
return sum([int(d) for d in numeric_string])

def valid_length_cc(cc_candidate):
return len(cc_candidate) == 16 or len(cc_candidate) == 15

def check_sum(numeric_string):
odds = numeric_string[:-1][::-2]
evens = numeric_string[::-2]
return sum([int(i) for i in odds]) + sum([sum_digits(str(int(i) * 2)) for i in even

Deprecation warning

This code works in both Python 2 and 3. However, in Python 3, unittest.assertEquals() is deprecated in favour of .assertEqual(), so use .assertEqual() for better compatibility.

Length check

valid_length_cc would be better written as return 15 <= len(cc_candidate) <= 16. It should also be moved further down, near the definition of is_valid_cc, since it has nothing to do with the Luhn algorithm code.

However, I recommend dispensing with the function altogether. The expected length varies according to the card type. You could just use the regex check to do the length validation for you.

Regex check

Your regex allows optional hyphens, but nowhere else in the code do you handle hyphens gracefully.

You incorrectly allow the second character of a card number that starts with 3 to be a comma.

Support for Discover cards appears to be incomplete.

The leading ^ anchor is redundant if you use re.match().

As previously mentioned, the regex should vary completely according to the card type. I would therefore write it this way for clarity:

def check_regex(candidate):
    PATTERN = re.compile(r'(4(?:\d{12}|\d{15})'   # Visa
                         r'|5[1-5]\d{14}'         # Mastercard
                         r'|6011\d{12}'           # Discover (incomplete)
                         r'|7\d{15}'              # Petroleum and misc.
                         r'|3[47]\d{13}'          # American Express
                         r')

Luhn algorithm check

In my opinion, is_luhn_valid() would be better named check_luhn(), to be consistent with check_regex().

find_check_digit() would be better named compute_check_digit().  You'll never "find" the check digit by looking for it.  I'm not sure that it's a useful function to have at all, though, as is_luhn_valid() should be good enough.

Rather than slicing the string in is_luhn_valid(), I recommend feeding it intact to check_sum(), since it's really part of the same checksumming kind of operation.  For compatibility with your existing code, I've introduced an includes_check_digit mode to check_sum() that defaults to False.

For simplicity, the sum_digits() function could be replaced by a lookup table.

When calling sum(), you only need a generator expression, not a list comprehension.  You can therefore write sum(… for …) instead of sum([… for …]).

Suggested implementation

import re

def check_sum(numeric_string, includes_check_digit=False):
    if not includes_check_digit:
        numeric_string += '0'
    EVEN_VALUES = [0, 2, 4, 6, 8, 1, 3, 5, 7, 9]
    evens = numeric_string[:-1][::-2]
    odds = numeric_string[::-2]
    return sum(int(i) for i in odds) + sum(EVEN_VALUES[int(i)] for i in evens)

def find_check_digit(numeric_string):
    return 9 * check_sum(numeric_string) % 10

def check_luhn(candidate):
    return candidate.isdigit() and check_sum(candidate, True) % 10 == 0

def check_regex(candidate):
    PATTERN = re.compile(r'(4(?:\d{12}|\d{15})'      # Visa
                         r'|5[1-5]\d{14}'            # Mastercard
                         r'|6011\d{12}'              # Discover (incomplete?)
                         r'|7\d{15}'                 # What's this?
                         r'|3[47]\d{13}'             # American Express
                         r'))
    return bool(PATTERN.match(candidate))


Luhn algorithm check

In my opinion, is_luhn_valid() would be better named check_luhn(), to be consistent with check_regex().

find_check_digit() would be better named compute_check_digit().  You'll never "find" the check digit by looking for it.  I'm not sure that it's a useful function to have at all, though, as is_luhn_valid() should be good enough.

Rather than slicing the string in is_luhn_valid(), I recommend feeding it intact to check_sum(), since it's really part of the same checksumming kind of operation.  For compatibility with your existing code, I've introduced an includes_check_digit mode to check_sum() that defaults to False.

For simplicity, the sum_digits() function could be replaced by a lookup table.

When calling sum(), you only need a generator expression, not a list comprehension.  You can therefore write sum(… for …) instead of sum([… for …]).

Suggested implementation

%%CODEBLOCK_1%%)
    return bool(PATTERN.match(candidate))

def is_valid_cc(candidate):
    return check_regex(candidate) and check_luhn(candidate)

) return bool(PATTERN.match(candidate))

Luhn algorithm check

In my opinion, is_luhn_valid() would be better named check_luhn(), to be consistent with check_regex().

find_check_digit() would be better named compute_check_digit(). You'll never "find" the check digit by looking for it. I'm not sure that it's a useful function to have at all, though, as is_luhn_valid() should be good enough.

Rather than slicing the string in is_luhn_valid(), I recommend feeding it intact to check_sum(), since it's really part of the same checksumming kind of operation. For compatibility with your existing code, I've introduced an includes_check_digit mode to check_sum() that defaults to False.

For simplicity, the sum_digits() function could be replaced by a lookup table.

When calling sum(), you only need a generator expression, not a list comprehension. You can therefore write sum(… for …) instead of sum([… for …]).

Suggested implementation

%%CODEBLOCK_1%%

StackExchange Code Review Q#74797, answer score: 6