2

So I am Having some problems when applying Luhn algorithm Here are the general rules: http://www.codeproject.com/Tips/515367/Validate-credit-card-number-with-Mod-algorithm

and here is my code

def luhn(credit_card)
  result = 0
    nums = credit_card.split("")
    nums.each_with_index do |item, index|
      if index.even?
        if item.to_i*2>9
            result+= item.to_i*2-9
        else 
            result+= item.to_i*2
        end
      else
        result +=item.to_i
        end
    end
    if (result % 10) == 0
      self.validation = "valid"
    else
      self.validation = "invalid"
    end
end

It works on the majority of cards

VISA: 4111111111111111       (valid)
VISA: 4111111111111          (invalid)
VISA: 4012888888881881       (valid)
Discover: 6011111111111117   (valid)
MasterCard: 5105105105105100 (valid)
MasterCard: 5105105105105106 (invalid)
Unknown: 9111111111111111    (invalid)

But when it comes to this one

AMEX: 37828224631000(invalid)

For some reason my code says its not valid,but it should be according to the official testing card list.

I have seen a bunch of other codes that are working but I want to correct the mistake and understand my mistake. I will appreciate some explanation why is it working like this.

Adrian Grzywaczewski
  • 868
  • 1
  • 12
  • 25

1 Answers1

2

Are you sure that your Amex number should be valid?

Can you edit your question to show us where you're getting your testing numbers?

Here's what I see in other Lunh test suites:

  • "378282246310005" should be true
  • "37828224631000" should be false

Also, here are items for you to look at:

  • Your code is moving through the numbers the wrong way: you're moving left-to-right, whereas Lunh is right-to-left.

  • Your code iterating on the check digit, whereas Lunh doesn't iterate on the check digit.

Try peeling off the check digit before you loop, and reversing your order, such as:

def luhn(credit_card)
  (*digits, checksum_digit) = s.split('').map(&:to_i)
  result = 0
  digits.reverse.each_with_index do |item, index|
    …

After you calculate the sum, then add the checksum digit, then compare % 10.

joelparkerhenderson
  • 34,808
  • 19
  • 98
  • 119
  • thank you @joelparkerhenderson, so it was just a coincidence that it was working for all other cards that i listed ? :( – Adrian Grzywaczewski Jan 13 '15 at 21:50
  • 1
    You're welcome. I don't think it's a coincidence, because that's so unlikely. I think it's because your code and the correct alorithm work the same when there are an even number of digits. – joelparkerhenderson Jan 13 '15 at 22:37