Luhn algorithm implementation

Question

I'm expecting luhn 5594589764218858 = True but it is always False

-- Get the last digit from a number
lastDigit :: Integer -> Integer
lastDigit 0 = 0
lastDigit n = mod n 10

-- Drop the last digit from a number
dropLastDigit :: Integer -> Integer
dropLastDigit n = div n 10

toRevDigits :: Integer -> [Integer]
toRevDigits n
  | n <= 0    = []
  | otherwise = lastDigit n : toRevDigits (dropLastDigit n)

-- Double every second number in a list starting on the left.
doubleEveryOther :: [Integer] -> [Integer]
doubleEveryOther []       = []
doubleEveryOther (x : []) = [x]
doubleEveryOther (x : y : z) = x : (y * 2) : doubleEveryOther z

-- Calculate the sum of all the digits in every Integer.
sumDigits :: [Integer] -> Integer
sumDigits []          = 0
sumDigits (x : [])    = x
sumDigits (x : y)     = (lastDigit x) + (dropLastDigit x) + sumDigits y

-- Validate a credit card number using the above functions.
luhn :: Integer -> Bool
luhn n
  | sumDigits (doubleEveryOther (toRevDigits n)) `div` 10 == 0 = True
  | otherwise                                                  = False

I know it can be done easier but I'm following a Haskell introductory. I think my only problem is in the luhn function. The course mentions problems may occur because toRevDigits reverses the number but I think it should work anyways.

I would suggest writing some tests for each of those components (a good opportunity to learn quickcheck, if you have a little time). — jberryman
– jberryman, Commented Jul 1, 2015 at 21:11
@BradyDean Your sumDigits function is wrong. Consider this example. lastDigit 123 returns 3. dropLastDigit 123 returns 12. However 3 + 12 is not equal to the sum of the digits (1 + 2 + 3). — jub0bs
– jub0bs, Commented Jul 1, 2015 at 21:14
The digits are being split up. sumDigits [10, 5, 18, 4] = 1 + 0 + 5 + 1 + 8 + 4 = 19 is what I'm going for — Brady Dean
– Brady Dean, Commented Jul 1, 2015 at 21:17
@BradyDean You're only getting a correct result in your example because all integers happen to have no more than two digits. Try sumDigits [1,100]. — jub0bs
– jub0bs, Commented Jul 1, 2015 at 21:44
@Jubobs Only handling values with at most two digits is sufficient in this case since doubleEveryOther will never yield numbers > 9*2. — Frerich Raabe
– Frerich Raabe, Commented Jul 1, 2015 at 23:10

Daniel Wagner · Accepted Answer · 2015-07-01 23:03:32Z

1

The snippet x `div` 10 == 0 is not a correct check that x is divisible by ten; you should use `mod` instead. Also, this equation is incorrect:

sumDigits (x : [])    = x

(Try, e.g. sumDigits [10].) It can be fixed, but deleting it is simpler and still correct.

answered Jul 1, 2015 at 23:03

Daniel Wagner

156k10 gold badges231 silver badges392 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Brady Dean Over a year ago

Wow I read over the code so many times and never noticed that. Code is working now with those changes.

Daniel Wagner Over a year ago

@BradyDean In my experience, you can't uncover bugs by reading code. And I didn't in this case, either; I ran snippets of your code on a your hand-crafted example to see what it would do, and whether that matched what I expected, looking for the smallest function that didn't match my expectations. This is a powerful technique that I encourage you to adopt.

Brady Dean Over a year ago

Yes I ran examples given in the course but I didn't have much to work with.

Collectives™ on Stack Overflow

Luhn algorithm implementation

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related