why it is returning only false? Luhn algorithm

73 views Asked by At

What's wrong with my code? It should return true for this array. The invalid one should return false. Please explain it to me because i'm just started with JS

//arrays :
const valid1 = [4, 5, 3, 9, 6, 7, 7, 9, 0, 8, 0, 1, 6, 8, 0, 8];
const invalid1 = [4, 5, 3, 2, 7, 7, 8, 7, 7, 1, 0, 9, 1, 7, 9, 5];

const validateCred = Array => {
    let cardNum = 0
    let reverseArray = Array.reverse()
    for (let i = 0; i < reverseArray.length; i++){

        let newVar = reverseArray[i]

       if (i%2 !== 0){
        newVar = reverseArray[i] * 2
            if (newVar > 9){
                newVar = newVar[i] - 9;
                cardNum += newVar
            } else {
                cardNum += newVar
            }
       } else {
        cardNum += reverseArray[i]
       }
    }
    return(cardNum%10 === 0 ? true : false)
}
console.log(validateCred(valid1))

1

There are 1 answers

9
Scott Sauyet On

As you figured out and noted in the comments, this is not going to go well when newVar is a number:

newVar = newVar[i] - 9;

And as Pointy, um, pointed out, Array is a terrible name for a variable, shadowing an important constructor function. More than that, there is a strong convention in JS that InitialCapital variable names are reserved for constructor functions. I would suggest a name that describes what it's for, not its type. Perhaps "creditCard" would be useful, or, depending on your tolerance for short abbreviations, "cc".

But there's another, more subtle, problem with this code. It alters its input:

const valid1 = [4, 5, 3, 9, 6, 7, 7, 9, 0, 8, 0, 1, 6, 8, 0, 8];
console .log (validateCred (valid1)) //=> true
console .log (valid1) //=> [8, 0, 8, 6, 1, 0, 8, 0, 9, 7, 7, 6, 9, 3, 5, 4]

In a real application, this could cause you all sorts of problems, and maybe far away from this section, always frustrating.

It's easy enough to fix. Just clone the array before reversing it. There are many ways to do it (using myVariable.slice() or myVariable.concat(), for instance.) My preference these days is to spread it into a new array: [...myVariable].


In my answer to another Luhn's Algorithm question, I developed what I think of as an elegant version of this algorithm. If you're new to JS, this may use some features you're not familiar with, but I find it clear and useful. This is a slightly improved version:

const luhn = (ds) => (
  [...ds]
    .filter(d => /^\d$/ .test (d))
    .reverse () 
    .map (Number) 
    .map ((d, i) => i % 2 == 1 ? (2 * d > 9 ? 2 * d - 9 : 2 * d) : d) 
    .reduce ((a, b) => a + b, 0) 
) % 10 == 0

It's the same algorithm, just expressed a little more concisely. Between the spreading of the initial value and the filter call (removing non-digits), it allows us to pass various input formats:

const valid1 = [4, 5, 3, 9, 6, 7, 7, 9, 0, 8, 0, 1, 6, 8, 0, 8];
const valid2 = "4539677908016808"
const valid3 = "4539-6779-0801-6808"
const valid4 = "4539 6779 0801 6808"