'why is this for loop not returning the intended output?

This is just an exercise in a course, this app selects a random fruit from the fruits array and then the removeItem() function is supposed to remove that one from the array and returns the modified array, but I'm getting a weird output, and the function is not working.

The problem can be seen here

function randomFruit(importedArray) {
  let random = Math.floor(Math.random() * importedArray.length);
  return importedArray[random];
}

function removeItem(importedArray, item) {
  for (let i = 0; i < importedArray.length; i++) {
    if (importedArray[i] === item) {
      importedArray.splice(i, 1);
      return [...importedArray.slice(0, i), ...importedArray.slice(i + 1)];
    } else {
      return "not found";
    }
  }
}

function makeFruitArray() {
  var foods = ["🍒", "🍉", "🍑", "🍐", "🍏"];
  return foods;
}

let fruitArray = makeFruitArray();
let fruitItem = randomFruit(fruitArray);
let remaining = removeItem(fruitArray, fruitItem);

console.log({fruitArray, fruitItem, remaining});


Solution 1:[1]

There were two major problems:

  • Your for loop in removeItem was always returning on the first iteration
  • You were modifying the initial array as you removed items from it

I've also removed all the unnecessary code used to reproduce your problem. Please read How to Ask to make sure you are helping others help you.

function randomFruit(importedArray) {
  let random = Math.floor(Math.random() * importedArray.length);
  return importedArray[random];
}

function removeItem(importedArray, item) {
  for (let i = 0; i < importedArray.length; i++) {
    if (importedArray[i] === item) {
      // Don't modify the original array since we want to
      // display the original fruits  
      return [...importedArray.slice(0, i), ...importedArray.slice(i + 1)];
    }
  }
  // Error condition goes outside the loop, not inside.
  return null;
}

function makeFruitArray() {
  var foods = ["?", "?", "?", "?", "?"];
  return foods;
}

let fruitArray = makeFruitArray();
let fruitItem = randomFruit(fruitArray);
let remaining = removeItem(fruitArray, fruitItem);

console.log({fruitArray, fruitItem, remaining});

Solution 2:[2]

There are two issues in the removeItem function -

  1. If the random one is not the first item on the array, the function returns not found. It wouldn't run for the second loop at all, as your function returns not found after the first iteration.

  2. The splice method updates the original array. While you are passing the fruitArray to the removeItem method, it gets passed as reference and updating it within the function using splice will update the actual array as well.

The simplest and safest way of removing an item from an array would be -

function removeItem(importedArray, item) {
  const filteredArray = importedArray.filter((each) => each !== item);
  if (filteredArray.length === 0) return 'Not Found';
  return filteredArray;
}

Solution 3:[3]

As himayan said, the issue was that splice already changes the array.

Here's my solution:

function removeItem(importedArray, item) {
  for (let i = 0; i < importedArray.length; i++) {
    if (importedArray[i] === item) {
      importedArray.splice(i, 1);
      break;
    }
  }

  return importedArray;
}

Solution 4:[4]

Your remove item function is not working correctly. Instead of writing loops and splicing the array to create a new one you should just use the filter method

function removeItem(importedArray, item) {
    let newArray = importedArray.filter(function (element) {
        return element !== item;
    });
    return newArray;
}

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1
Solution 2
Solution 3 Ron Hillel
Solution 4 Vikrant Kashyap