Working on a site that must validate a credit card number. One of my collaborators suggested this script. Clearly, before using some rando's credit card validating script, one should take a look-see and make sure it doesn't steal credit card numbers. I've looked at this script pretty hard and do not detect any theft type stuff, but I am put off by quite a bit of pedantry. For instance, line 72:

for (n = k = ref = r[1], ref1 = r[2]; ref <= ref1 ? k <= ref1 : k >= ref1; n = ref <= ref1 ? ++k : --k) {
            trie.push(n);
}

What kind of jerk writes a for loop like that? There are also quite a few anonymous functions and other weirdness.

All it appears to do is check if the supplied cardnumber is a luhn number and then checks certain constraints based on a match of the opening sequence of letters.

A couple of questions then:
1) Does anyone have a favorite credit card validation script they use? Please suggest!
2) Does anyone see any evil sneakiness in this script that might facilitate theft of cardholder info?
3) Is anyone else put off by the coding in this script like I am?
4) Are all credit cards really luhn numbers? It would be quite regrettable to exclude a valid card number that is not a luhn number.
5) Does anyone have any comments on the card validation criteria starting at line 121?

Any feedback would be much appreciated.

    A Luhn number test is simply an integrity test, ensuring that the user didn't transpose digits, etc., in their typing. Here's a very simple PHP implementation that could easily be ported to JS (since I think that's where I got it):

    function isCreditCard($CC)  {

$l = strlen($CC);

if ($l > 19) {

    echo("You have entered an invalid card");
    return false;
}

$sum = 0;
$mul = 1;

for ($i = 0; $i < $l; $i++ ) {

    $digit    = substr($cc, $l-$i-1,$l-$i);
    $tproduct = intval($digit)*$mul;

    if ($tproduct >= 10)
        $sum = ($tproduct % 10)*1;
    else
        $sum = $tproduct;

    if ($mul == 1) {
                     $mul++;
    } else {
                     $mul--;
    }
}

if (($sum % 10) == 0) {
            echo("Congrats you entered a valid card number");
             return true;
} else {
            echo("You have entered an invalid card number");
             return false;
}
}

    We only accept certain CCards, so our tests also involve evaluation of the length of the string and the first $n digits to assure they are MasterCard, Maestro, VISA, AMEX, etc. I see some of that around line 121, but he's attempting to implement a kitchen sink solution, it seems.

    What kind of jerk writes a for loop like that? There are also quite a few anonymous functions and other weirdness.[/code]

    Ah, JavaScript. Its reputation is well-deserved.

      The loop initialiser assigns "r[1]" to "ref" and "r[2]" to "ref1": that's already pretty sad.
      Oh, and n always equals k - one of those variables is redundant. This is feeling like "throw stuff at the wall until something sticks" coding.

      And personally, I'd check whether ref <= ref1 or not once to decide whether to loop forwards or backwards, instead of checking on every iteration....

      But then I'd look at what I was doing and realise that the order in which I added items to the trie is irrelevant and I could just swap the loop bounds and iterate in one direction.

      Assuming I don't realise the trie is a way overblown structure for something that could be handled by a plain JavaScript array and its includes() method.

      (sigh)

        8 days later

        Thanks for your suggestions and thanks even more for your remarks disparaging the ugly code. After staring at it way too long, I extracted the Luhn number check and the code that checks card types and wrote a much cleaner and greatly simplified couple of functions that I could actually use in my context. I think CoffeeScript is partly to blame for that code.

          Write a Reply...