HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Haskell code to verify credit number

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numbercreditverifyhaskellcode

Problem

I am new to Haskell, and wrote a script to verify credit number.
I did some tests, the script worked, but can it be improved further?

isCreditCardNumber :: String -> Bool
isCreditCardNumber number =
    0 == creditCardReminder ( creditCardDouble ( blowupCreditCardNumber ( reverse number)))

blowupCreditCardNumber :: String -> [(Int,Char)]
blowupCreditCardNumber creditCardNumber = zip [1..] creditCardNumber

creditCardDouble :: [(Int,Char)] -> [Int]
creditCardDouble [] = []
creditCardDouble ((index,digit):rest)
    | even index = (numberDoubleToList ((*2) $ digitToInt digit)) ++ creditCardDouble rest
    | otherwise = digitToInt digit : creditCardDouble rest

numberDoubleToList:: Int -> [Int]
numberDoubleToList number
    | number > 9 = map digitToInt (show number)
    | otherwise  = [number]

creditCardReminder :: [Int] -> Int
creditCardReminder xs = sum xs `mod` 10

Solution

You can eta-reduce blowupCreditCardNumber:

blowupCreditCardNumber = zip [1..]


Using composition, you can also make isCreditCardNumber pointfree:

isCreditCardNumber = (==0) . creditCardReminder . creditCardDouble . blowupCreditCardNumber . reverse


You don't need to optimize numberDoubleToList by special-casing the one-digit case.

numberDoubleToList = map digitToInt . show


The explicit recursion in creditCardDouble can be averted by using library functions that specialize in particular recursive patterns:

creditCardDouble = concatMap foo where
  foo (index, digit) | even index = numberDoubleToList $ (*2) $ digitToInt digit
                     | otherwise = [digitToInt digit]


foo's name makes foo look like a crutch, and that is good, because it is one.

I would inline definitions that are only used once and do not deserve to be in a library:

isCreditCardNumber = (==0) . (`mod` 10) . sum . concatMap foo . zip [1..] . reverse where
  foo (index, digit) | even index = map digitToInt $ show $ (*2) $ digitToInt digit
                     | otherwise = [digitToInt digit]


digitToInt digit is used in both cases of foo, and so can be factored out:

isCreditCardNumber = (==0) . (`mod` 10) . sum . concatMap foo . zip [1..] . reverse where
  foo (index, digit) = bar index $ digitToInt digit
  bar index | even index = map digitToInt . show  . (*2)
            | otherwise = pure


In fact, we don't need to generate the index and pass it to foo if all we do with it is put it into bar later:

isCreditCardNumber = (==0) . (`mod` 10) . sum . concatMap foo . zip (cycle [pure, map digitToInt . show . (*2)]) . reverse where
  foo (doubler, digit) = doubler $ digitToInt digit


foo is almost trivial, lets get rid of it entirely:

isCreditCardNumber = (==0) . (`mod` 10) . sum . concat . zipWith ($) (cycle [pure, map digitToInt . show . (*2)]) . map digitToInt . reverse


map digitToInt . show is pure on single digits, so we can factor it out of that list and then even factor out the (*):

isCreditCardNumber = (==0) . (`mod` 10) . sum . concatMap (map digitToInt . show) . zipWith (*) (cycle [1, 2]) . map digitToInt . reverse


Note that if you know the parity of the length of the credit card number, you can get rid of the reverse.

Code Snippets

blowupCreditCardNumber = zip [1..]
isCreditCardNumber = (==0) . creditCardReminder . creditCardDouble . blowupCreditCardNumber . reverse
numberDoubleToList = map digitToInt . show
creditCardDouble = concatMap foo where
  foo (index, digit) | even index = numberDoubleToList $ (*2) $ digitToInt digit
                     | otherwise = [digitToInt digit]
isCreditCardNumber = (==0) . (`mod` 10) . sum . concatMap foo . zip [1..] . reverse where
  foo (index, digit) | even index = map digitToInt $ show $ (*2) $ digitToInt digit
                     | otherwise = [digitToInt digit]

Context

StackExchange Code Review Q#144929, answer score: 11

Revisions (0)

No revisions yet.