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

Haskell Luhn Algorithm

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

Problem

Trying to get to grips with Haskell. The problem is taken from here.

{-
Given Credit Card number: 49927398716 

• Reverse the digits: 61789372994 
• Sum the odd digits: 6 + 7 + 9 + 7 + 9 + 4 = 42 = s1 
• The even digits: 1, 8, 3, 2, 9 
•  Two times each even digit: 2, 16, 6, 4, 18 
•  Sum the digits of each multiplication: 2, 7, 6, 4, 9 
•  Sum the last: 2 + 7 + 6 + 4 + 9 = 28 = s2
• s1 + s2 = 70 
• which, as it ends in zero, means that 49927398716 passes the Luhn test
-}

import Data.Char

data Validity = Valid | Invalid deriving Eq
type CardNumber = String

luhnCheck :: CardNumber -> Validity
luhnCheck s = 
    let r = revDigits s
        (os,es) = oddsAndEvens r
        s1 = sum os
        s2 = sum $ map sumDigits $ map (*2) es 
    in if (s1 + s2) `mod` 10 == 0 then Valid
       else Invalid

revDigits :: CardNumber -> [Int]
revDigits s = map digitToInt (reverse s)    

oddsAndEvens :: [Int] -> ([Int],[Int])
oddsAndEvens is = 
    let digitsWithIdx = zip is (cycle [1,2])
        odds = map fst $ filter (\d -> (snd d) == 1) digitsWithIdx
        evens = map fst $ filter (\d -> (snd d) == 2) digitsWithIdx
    in (odds,evens)     

sumDigits:: Int -> Int
sumDigits i = sum $ map digitToInt $ show i

test :: Bool
test = let testData = ["49927398716", "49927398717", "1234567812345678","1234567812345670"]
    in  (map luhnCheck testData) == [Valid,Invalid,Invalid,Valid]

Solution

One thing that immediately pops out (and happens a lot with beginner Haskell programmers, IME):

s2 = sum $ map sumDigits $ map (*2) es


This means (reads as for me) "map (*2) on es, then map sumDigits on it, then sum the numbers."

What you typically want to do is to stay on the "function side" on things as long as possible:

s2 = sum . map sumDigits . map (*2) $ es


Meaning you compose as much as you can, and put only one application at the end. That makes it easier to eta-reduce and in general is considered more idiomatic.

You could also compose the mapped operation, bringing it down to classic "map-reduce" scenario (where "map" part is sumDigits . (*2), and "reduce" part is the sum).

s2 = sum . map (sumDigits . (*2)) $ es


I'd also flip the order of Valid and Invalid:

data Validity = Invalid | Valid deriving Eq


This way, when you derive Ord, Valid > Invalid, which is sometimes helpful and in general you'll see that it holds "intuitively", for Maybe (Just _ > Nothing), Either (Right _ > Left _) and similar values.

Code Snippets

s2 = sum $ map sumDigits $ map (*2) es
s2 = sum . map sumDigits . map (*2) $ es
s2 = sum . map (sumDigits . (*2)) $ es
data Validity = Invalid | Valid deriving Eq

Context

StackExchange Code Review Q#81810, answer score: 2

Revisions (0)

No revisions yet.