0

Today I faced with a problem and I fixed it but I didn't know why it occurs! So I decided to ask it here.

Let me show my problem fast and short:

function newProp(array) {
  let returnArray = [];
  for (i in array) {
    returnArray.push(i);
  }
  return returnArray;
}

let underTest = [{
  username: 'stackoverflow',
  email: 'stackoverflow@noStack.com',
  total: 2
}]
let input = [1, 2, 3, 4];

for (i in underTest) {
  console.log('before call:', underTest[i]);
  underTest[i].permutation = newProp(input);
  console.log('after call the underTest[i] does not exist:', underTest[i]);
  console.log('But the whole array still exists', underTest);
}

Code Explanation: I'm trying to add a new property called permutation to each object of underTest array.

Problem: I don't expect the underTest[i] be undefined after calling newProp() function.

My Question: Why underTest[i] is undefined after calling newProp()? I think it's because of using for in over array in my code but in MDN web doc there is no hint about my problem, it just says:

Note: for...in should not be used to iterate over an Array where the index order is important.

Array indexes are just enumerable properties with integer names and are otherwise identical to general object properties. There is no guarantee that for...in will return the indexes in any particular order. The for...in loop statement will return all enumerable properties, including those with non–integer names and those that are inherited.

Because the order of iteration is implementation-dependent, iterating over an array may not visit elements in a consistent order. Therefore, it is better to use a for loop with a numeric index (or Array.prototype.forEach() or the for...of loop) when iterating over arrays where the order of access is important.

and due to above paragraphs while I don't mind the order of elements I should be able to use for in structure(I don't mind the inherited properties too).

Solutions: By trial and error, I found so many solutions to solve the problem and I think it's useful to mention some of them:

  1. Replace for in with a traditional for loop will solve the problem:

function newProp(array) {
  let returnArray = [];
  for (i in array) {
    returnArray.push(i);
  }
  return returnArray;
}

let underTest = [{
  username: 'stackoverflow',
  email: 'stackoverflow@noStack.com',
  total: 2
}]
let input = [1, 2, 3, 4];

for (let i = 0; i < underTest.length; i++) {
  console.log('before call:', underTest[i]);
  underTest[i].permutation = newProp(input);
  console.log('after call the underTest[i] exists', underTest[i]);
}
  1. In newProp functions, If Instead of making the array dynamically, return an static array then the problem disappears(I didn't know why, yet!):

function newProp(array) {
  return [1, 2, 3, 4];
}

let underTest = [{
  username: 'stackoverflow',
  email: 'stackoverflow@noStack.com',
  total: 2
}]
let input = [1, 2, 3, 4];

for (i in underTest) {
  console.log('before call:', underTest[i]);
  underTest[i].permutation = newProp(input);
  console.log('after call the underTest[i] exists', underTest[i]);
}
  1. you can use built-in forEach method of underTest array and it will solve the problem too but I didn't like it because it can leads to callback hell problem(especially if you work with asynchronous jobs). So I didn't provide its snippet.

NOTE: If you can, please explain why the second solution will solve the problem too?

milad
  • 1,854
  • 2
  • 11
  • 27
  • 2
    Your `i` is global which you continually overwrite, this after you call `newProp`, it will have the last value it had after the loop in that function. Just declare it, so it's not implicitly global. – VLAZ Oct 20 '20 at 09:34
  • 2
    You need to add `let`: `for (let i in underTest)` – Hao Wu Oct 20 '20 at 09:35
  • Normally we use ```for...in``` for objects and not for arrays.. Some people considering it as a bad practice to use it for array.. https://stackoverflow.com/q/500504/7785337 – Maniraj Murugan Oct 20 '20 at 09:36
  • @VLAZ Thank you both of you I even couldn't imagine my problem was so simple. – milad Oct 20 '20 at 09:37
  • @HaoWu Thank you, I even couldn't imagine my problem was so simple – milad Oct 20 '20 at 09:37
  • @ManirajMurugan this is no problem to use it here, read my complete question – milad Oct 20 '20 at 09:38
  • @milad, I am not complaining anything about your question.. It is fine.. But coming to best practice its recommended to use ```for...in``` only for objects.. – Maniraj Murugan Oct 20 '20 at 09:39
  • Ref: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in – Maniraj Murugan Oct 20 '20 at 09:40
  • @ManirajMurugan you're right, I'll try to avoid it next times. thank you. – milad Oct 20 '20 at 09:42

1 Answers1

3

As @HaoWu and @VLAZ commented, My problem is not related to for in loop. It's because of declaring the i variables as global.

if I change the declaration of i variables to local the problem will disappear:

function newProp(array) {
  let returnArray = [];
  for (let i in array) {
    returnArray.push(i);
  }
  return returnArray;
}

let underTest = [{
  username: 'stackoverflow',
  email: 'stackoverflow@noStack.com',
  total: 2
}]
let input = [1, 2, 3, 4];

for (let i in underTest) {
  console.log('before call:', underTest[i]);
  underTest[i].permutation = newProp(input);
  console.log('after call the underTest[i] exists:', underTest[i]);
}

Updates:

As @3limin4t0r mentioned below this answer: if you don't need the index of elements it's better to use for of structure instead of for in:

function newProp(array) {
  let returnArray = [];
  for (let i in array) {
    returnArray.push(i);
  }
  return returnArray;
}

let underTest = [{
  username: 'stackoverflow',
  email: 'stackoverflow@noStack.com',
  total: 2
}]
let input = [1, 2, 3, 4];

for (let element of underTest) {
  console.log('before call:', element);
  element.permutation = newProp(input);
  console.log('after call the underTest[i] exists:', element);
}
milad
  • 1,854
  • 2
  • 11
  • 27
  • You also don't need an `i` if you where to use [`for...of`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of) instead of [`for...in`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in) to iterate arrays. – 3limin4t0r Oct 20 '20 at 09:53
  • Why use `for in` at all. I have hard time finding a reason why to use it in general. Use ES6 `forEach` or `map` instead. – Max Favilli Oct 20 '20 at 09:56
  • @3limin4t0r You're right. but this code is a simplified version of main code in a big project which we need the index of each element later too. but if there was no need to indexes of elements I prefer to use the `for...of` too. – milad Oct 20 '20 at 10:01
  • @MaxFavilli unfortunately `forEach` and `map` doesn't support `ansyc` functions as callback: if you should `await` to some asynchronous jobs in your callback you should define the callback as `async` but then the order of elements is almost random(so you couldn't find when the callback finishes). By the way the best solution to fix this, is using `map` and `Promise.all` together which is out of context here. – milad Oct 20 '20 at 10:09