array doesn't push expected element

Question

I'm learning JavaScript and having this problem. Basically I have to check for invalid credit cards number

// All valid credit card numbers
const valid1 = [4, 5, 3, 9, 6, 7, 7, 9, 0, 8, 0, 1, 6, 8, 0, 8]
const valid2 = [5, 5, 3, 5, 7, 6, 6, 7, 6, 8, 7, 5, 1, 4, 3, 9]
const valid3 = [3, 7, 1, 6, 1, 2, 0, 1, 9, 9, 8, 5, 2, 3, 6]
const valid4 = [6, 0, 1, 1, 1, 4, 4, 3, 4, 0, 6, 8, 2, 9, 0, 5]
const valid5 = [4, 5, 3, 9, 4, 0, 4, 9, 6, 7, 8, 6, 9, 6, 6, 6]

// All invalid credit card numbers
const invalid1 = [4, 5, 3, 2, 7, 7, 8, 7, 7, 1, 0, 9, 1, 7, 9, 5]
const invalid2 = [5, 7, 9, 5, 5, 9, 3, 3, 9, 2, 1, 3, 4, 6, 4, 3]
const invalid3 = [3, 7, 5, 7, 9, 6, 0, 8, 4, 4, 5, 9, 9, 1, 4]
const invalid4 = [6, 0, 1, 1, 1, 2, 7, 9, 6, 1, 7, 7, 7, 9, 3, 5]
const invalid5 = [5, 3, 8, 2, 0, 1, 9, 7, 7, 2, 8, 8, 3, 8, 5, 4]

// Can be either valid or invalid
const mystery1 = [3, 4, 4, 8, 0, 1, 9, 6, 8, 3, 0, 5, 4, 1, 4]
const mystery2 = [5, 4, 6, 6, 1, 0, 0, 8, 6, 1, 6, 2, 0, 2, 3, 9]
const mystery3 = [6, 0, 1, 1, 3, 7, 7, 0, 2, 0, 9, 6, 2, 6, 5, 6, 2, 0, 3]
const mystery4 = [4, 9, 2, 9, 8, 7, 7, 1, 6, 9, 2, 1, 7, 0, 9, 3]
const mystery5 = [4, 9, 1, 3, 5, 4, 0, 4, 6, 3, 0, 7, 2, 5, 2, 3]

// An array of all the arrays above
const batch = [valid1, valid2, valid3, valid4, valid5, invalid1, invalid2, invalid3, invalid4, invalid5, mystery1, mystery2, mystery3, mystery4, mystery5]

The validity of a credit card is accessed by this algorithm (Luhn algorithm):

function validateCred(card_number){
let sum = card_number[card_number.length-1];
let flag = 1;
for (i = card_number.length-2; i >= 0; i--){
    if (flag % 2 !== 0){
        card_number[i] *= 2;
        if (card_number[i] > 9){
            card_number[i] -= 9;
        }
    }
    sum += card_number[i];
    flag++;
}

if (sum % 10 === 0){
    return true;
} else {
    return false;
}
}

And I have to create an array with all the invalid cards:

function findInvalidCards(numbers){
    let invalid_array = [];
    for (i = 0; i < numbers.length; i++){
        if (!validateCred(numbers[i])){
            invalid_array.push(numbers[i]);
        }
    }
    return invalid_array;
}

When I call the findInValidCards function, it raises the heap out of memory error, I tried to follow the solution from this link and raise the usage memory to 8gb but the problem persists. After debugging, I found out that this line invalid_array.push(numbers[i]) actually appends a undefined variable to the array, not the element that I wanted. What could possibly cause this problem?

The code you've posted shouldn't produce any of the behaviour you describe - there shouldn't be an out of memory error, nor should `numbers[i]` ever resolve to `undefined`. However, the behaviour does imply that something else is modifying the `numbers` array and you might even be calling this recursively or in a loop or something. Your description hints at the problem being elsewhere. — VLAZ, Dec 29 '19 at 17:55
you need to declare `i` for each function. otherwise `i` changes the value erraticly. — Nina Scholz, Dec 29 '19 at 18:00
to elaborate on what @NinaScholz said, try `for (var i =...` or `for (let i =...` — Robb Traister, Dec 29 '19 at 18:15

Shiny · Accepted Answer · 2019-12-29T18:24:14.900

As Nina had said in the comments, you should be defining i with let each time you're writing the for loop or else you're going to have unpredictable behavior

However, you also had an issue in your validateCred function where you were mutating the original Array while checking it - you can simply clone the input Array with let clone = card_number.slice(0);, and then reference clone for your checks

Also, you can simplify findInvalidCards by using .filter(), like:

function findInvalidCards(numbers) {
  return numbers.filter(x => !validateCred(x));
}

// All valid credit card numbers
const valid1 = [4, 5, 3, 9, 6, 7, 7, 9, 0, 8, 0, 1, 6, 8, 0, 8];
const valid2 = [5, 5, 3, 5, 7, 6, 6, 7, 6, 8, 7, 5, 1, 4, 3, 9];
const valid3 = [3, 7, 1, 6, 1, 2, 0, 1, 9, 9, 8, 5, 2, 3, 6];
const valid4 = [6, 0, 1, 1, 1, 4, 4, 3, 4, 0, 6, 8, 2, 9, 0, 5];
const valid5 = [4, 5, 3, 9, 4, 0, 4, 9, 6, 7, 8, 6, 9, 6, 6, 6];

// All invalid credit card numbers
const invalid1 = [4, 5, 3, 2, 7, 7, 8, 7, 7, 1, 0, 9, 1, 7, 9, 5];
const invalid2 = [5, 7, 9, 5, 5, 9, 3, 3, 9, 2, 1, 3, 4, 6, 4, 3];
const invalid3 = [3, 7, 5, 7, 9, 6, 0, 8, 4, 4, 5, 9, 9, 1, 4];
const invalid4 = [6, 0, 1, 1, 1, 2, 7, 9, 6, 1, 7, 7, 7, 9, 3, 5];
const invalid5 = [5, 3, 8, 2, 0, 1, 9, 7, 7, 2, 8, 8, 3, 8, 5, 4];

// Can be either valid or invalid
const mystery1 = [3, 4, 4, 8, 0, 1, 9, 6, 8, 3, 0, 5, 4, 1, 4];
const mystery2 = [5, 4, 6, 6, 1, 0, 0, 8, 6, 1, 6, 2, 0, 2, 3, 9];
const mystery3 = [6, 0, 1, 1, 3, 7, 7, 0, 2, 0, 9, 6, 2, 6, 5, 6, 2, 0, 3];
const mystery4 = [4, 9, 2, 9, 8, 7, 7, 1, 6, 9, 2, 1, 7, 0, 9, 3];
const mystery5 = [4, 9, 1, 3, 5, 4, 0, 4, 6, 3, 0, 7, 2, 5, 2, 3];

// An array of all the arrays above
const batch = [valid1, valid2, valid3, valid4, valid5, invalid1, invalid2, invalid3, invalid4, invalid5, mystery1, mystery2, mystery3, mystery4, mystery5];

function validateCred(card_number) {
  let clone = card_number.slice(0);

  let sum = clone[clone.length - 1];
  let flag = 1;

  for (i = clone.length - 2; i >= 0; i--) {
    if (flag % 2 !== 0) {
      clone[i] *= 2;
      if (clone[i] > 9) {
        clone[i] -= 9;
      }
    }

    sum += clone[i];
    flag++;
  }

  if (sum % 10 === 0) {
    return true;
  } else {
    return false;
  }
};

// And I have to create an array with all the invalid cards:
function findInvalidCards(numbers) {
  let invalid_array = [];
  for (let i = 0; i < numbers.length; i++) {
    if (!validateCred(numbers[i])) {
      invalid_array.push(numbers[i]);
    }
  }
  return invalid_array;
}

let invalidCardArray = findInvalidCards(batch);

// Prettier console.log()
invalidCardArray.forEach(x => console.log(validateCred(x) + ':', ...x));

It worked, thanks. Can you elaborate more about why doesn't it throw an error if I don't define "i" for each loop? — duy anh hoang, Dec 29 '19 at 18:26
Without defining `i` it's added to the global scope, so both of your for loops are changing the same variable - When you use [`let`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let) you're scoping it only to that block of code, so this crossover doesn't happen — Shiny, Dec 29 '19 at 18:28

Marios Nikolaou · Answer 2 · 2019-12-29T18:42:21.590

The i variable is defined as global if you don't define it with var or let. So when it starts from findInvalidCards function the i value is zero but when it access the validateCred function, the i value changes.So when it will finish from validateCred function, i value will be -1 and it will try to increment it in findInvalidCards for loop and i value will be zero.That will happen over and over again,it will go in infinite loop causing the memory exception and everything crashes.

I have debug your code, so i know how it goes. :)

  function findInvalidCards(numbers){
        let invalid_array = [];
        //make i local variable by defining it let or var, otherwise it will be global
        for (var i = 0; i < numbers.length; i++){
            if (!validateCred(numbers[i])){
                invalid_array.push(numbers[i]);
            }
        }
        return invalid_array;
    }

function validateCred(card_number){
    let sum = card_number[card_number.length-1];
    let flag = 1;
    //make i local variable by defining it let or var, otherwise it will be global
    for (var i = card_number.length-2; i >= 0; i--){
        if (flag % 2 !== 0){
            card_number[i] *= 2;
            if (card_number[i] > 9){
                card_number[i] -= 9;
            }
        }
        sum += card_number[i];
        flag++;
    }

    if (sum % 10 === 0){
        return true;
    } else {
        return false;
    }
}

array doesn't push expected element

2 Answers2