Remove item from Javascript array and display new array

Question

This simple code allows me to display a list of items in an array, and to delete a chosen item from the array, and finally display a new list without the deleted item.

var carBrands = ["Toyota", "Honda", "BMW", "Lexus", "Mercedes","Peugeot","Aston   Martin","Rolls Royce"];
var html="";
var newList="";
var itemToRemove;

$(document).ready (function () {
console.log("Ready to go!");
$("#displayList").bind('click', function(event) {
    displayList();
   });
$("#removeItem").bind('click', function(event) {
    item = document.getElementById("input").value;
    removeItemFromList(item);
});
 });
function displayList() {
for (var i = 0; i < carBrands.length; i++) {
    html+= "<li>" + carBrands[i] + "</li>";
    document.getElementById("list").innerHTML=html;
}

}
function removeItemFromList(item) {
itemToRemove = item;

for (var i=0; i<carBrands.length; i++) {
    if (itemToRemove == carBrands[i]) {
        carBrands.splice(carBrands[i], 1);
    }
    newList+= "<li>" + carBrands[i] + "</li>";
    document.getElementById("newList").innerHTML=newList;
}
}

This works fine on the first try,

but if i try again the new list is appended to the old list and the item i removed initially is also added to it.

My Question:

1. How do i display the list without the removed items?
2. Are html and newList objects?

Answer 1

Your problem has to do with variable scope: the html and newList variables retained their values each time the functions were run (aka: each time the buttons were pressed).

You also had a severe performance issue. When looping, you don't want to update the DOM on each iteration, and you want to update the DOM as few times as is necessary.

Don't do:

var html = "";
function displayList() {
    html = "";
    for (var i = 0; i < carBrands.length; i++) {
        html += "<li>" + carBrands[i] + "</li>";
        document.getElementById("list").innerHTML = html;
    }    
}

Do:

function displayList() {
    var html = "";
    for (var i = 0; i < carBrands.length; i++) {
        html += "<li>" + carBrands[i] + "</li>";
    }
    document.getElementById("list").innerHTML = html;
}

Here is my jsfiddle of the solution: http://jsfiddle.net/netinept/jmm93fy1/

And the complete resulting JavaScript (including Paul Roub's join solution for constructing the final list):

var carBrands = ["Toyota", "Honda", "BMW", "Lexus", "Mercedes", "Peugeot", "Aston Martin", "Rolls Royce"];


var itemToRemove;

$(document).ready(function () {
    console.log("Ready to go!");
    $("#displayList").bind('click', function (event) {
        displayList();
    });
    $("#removeItem").bind('click', function (event) {
        item = document.getElementById("input").value;
        removeItemFromList(item);
    });
});

function displayList() {
    var html = "";
    for (var i = 0; i < carBrands.length; i++) {
        html += "<li>" + carBrands[i] + "</li>";
    }
    document.getElementById("list").innerHTML = html;
}

function removeItemFromList(item) {
    itemToRemove = item;
    for (var i = 0; i < carBrands.length; i++) {
        if (itemToRemove == carBrands[i]) {
            carBrands.splice(i, 1);
            break;
        }
    }
    var newList = "<li>" + carBrands.join('</li><li>') + "</li>";
    document.getElementById("newList").innerHTML = newList;
}

Answer 2

First, splice() doesn't want the thing you're removing, it wants the index of the thing.

So, instead of:

carBrands.splice(carBrands[i], 1);

you want:

carBrands.splice(i, 1);

And you're never emptying newList after deleting the last item, so you're just re-adding everything to the list.

Try this instead:

function removeItemFromList(item) {
  itemToRemove = item;

  for (var i=0; i < carBrands.length; i++) {
    if (item == carBrands[i]) {
      carBrands.splice(i, 1);
      break;
    }
  }

  newList = "<li>" + carBrands.join('</li><li>') + "</li>";  
  document.getElementById("newList").innerHTML = newList;
}

Also, you'll notice that itemToRemove was never needed in the loop; if you're not using it elsewhere, it can just go away.