folks! Today I created this script that has the following functionality:
There are two functions:
The code includes 3 for loops and you can see it at - http://jsfiddle.net/menian/3b4qp/1/
My Master told me that those 3 for loops make the solution way to heavy. Is there a better way to do the same thing? Is it better to decrease the loops and try to use splice? Thanks in advance.
HTML
<!-- we add to our foodList from the value of the following input -->
<input type="text" value="food" id="addFood" />
<!-- we call addToFood(); through the following button -->
<input type="submit" value="Add more to food" onClick="addToFood();">
<!-- The list of food is displayed in the following div -->
<div id="foods"></div>
JavaScript
var foodList = [];
function addToFood () {
var addFood = document.getElementById('addFood').value;
foodList.push(addFood);
for (i = 0; i < foodList.length; i++) {
var newFood = "<a href='#' onClick='removeRecord(" + i + ");'>X</a> " + foodList[i] + " <br>";
};
document.getElementById('foods').innerHTML += newFood;
}
function removeRecord (i) {
// define variable j with equal to the number we got from removeRecord
var j = i;
// define and create a new temporary array
var tempList = [];
// empty newFood
// at the end of the function we "refill" it with the new content
var newFood = "";
for (var i = 0; i < foodList.length; i++) {
if(i != j) {
// we add all records except the one == to j to the new array
// the record eual to j is the one we've clicked on X to remove
tempList.push(foodList[i]);
}
};
// make redefine foodList by making it equal to the tempList array
// it should be smaller with one record
foodList = tempList;
// re-display the records from foodList the same way we did it in addToFood()
for (var i = 0; i < foodList.length; i++) {
newFood += "<a href='#' onClick='removeRecord(" + i + ");'>X</a> " + foodList[i] + " <br>";
};
document.getElementById('foods').innerHTML = newFood;
}
Pushing a value onto an array a is the same as assigning the value to a[a. length] . You can use the unshift() method (described in Array Methods) to insert a value at the beginning of an array, shifting the existing array elements to higher indexes.
To remove an object from an array by its value: Call the findIndex() method to get the index of the object in the array. Use the splice() method to remove the element at that index. The splice method changes the contents of the array by removing or replacing existing elements.
The splice() method allows you to insert new elements into an array while deleting existing elements simultaneously.
Array.prototype.splice() The splice() method changes the contents of an array by removing or replacing existing elements and/or adding new elements in place.
You should use array.splice(position,nbItems)
function removeRecord (i) {
foodList.splice(i, 1); // remove element at position i
var newFood = "";
for (var i = 0; i < foodList.length; i++) {
newFood += "<a href='#' onClick='removeRecord(" + i + ");'>X</a> "
+ foodList[i] + " <br>";
};
document.getElementById('foods').innerHTML = newFood;
}
http://jsfiddle.net/3b4qp/5/
Now using JQuery:
$(function(){
$(document).on('click','input[type=submit]',function(){
$('#foods')
.append('<div><a href="#" class="item">X</a> '
+ $('#addFood').val() + '</div>');
});
$(document).on('click','.item',function(){
$(this).parent().remove();
});
});
http://jsfiddle.net/jfWa3/
Your problem isn't the arrays, your problem is this code:
node.innerHTML += newFood;
This code is very, very, very slow. It will traverse all exising DOM nodes, create strings from them, join those strings into one long string, append a new string, parse the result to a new tree of DOM nodes.
I suggest to use a framework like jQuery which has methods to append HTML fragments to existing DOM nodes:
var parent = $('#foods');
...
for (var i = 0; i < foodList.length; i++) {
parent.append( "<a href='#' onClick='removeReco..." );
That will parse the HTML fragments only once.
If you really must do it manually, then collect all the HTML in a local string variable (as suggested by JohnJohnGa in his answer) and then assign innerHTML
once.
Here's some tips to, at least, make your code more portable (dunno if it will be better performance wise, but should be, since DOM Manipulation is less expensive)
FIDDLE
<div id="eventBinder">
<!-- we add to our foodList from the value of the following input -->
<input id="addFood" type="text" value="food" />
<!-- we call addToFood(); through the following button -->
<button id="addFoodBtn" value="Add more to food">Add Food</button>
<!-- The list of food is displayed in the following div
-->
<div id="foods"></div>
</div>
// FoodList Class
var FoodList = function (selectorID) {
return {
foodArray: [],
listEl: document.getElementById(selectorID),
idCnt: 0,
add: function (newFood) {
var id = 'myfood-' + this.idCnt;
this.foodArray.push({
id: id,
food: newFood
});
var foodDom = document.createElement('div'),
foodText = document.createTextNode(newFood);
foodDom.setAttribute('id', id);
foodDom.setAttribute('class', 'aFood');
foodDom.appendChild(foodText);
this.listEl.appendChild(foodDom);
++this.idCnt;
},
remove: function (foodID) {
for (var f in this.foodArray) {
if (this.foodArray[f].id === foodID) {
delete this.foodArray[f];
var delFood = document.getElementById(foodID);
this.listEl.removeChild(delFood);
}
}
}
};
};
//Actual app
window.myFoodList = new FoodList('foods');
document.getElementById('eventBinder').addEventListener('click', function (e) {
if (e.target.id === 'addFoodBtn') {
var food = document.getElementById('addFood').value;
window.myFoodList.add(food);
} else if (e.target.className === 'aFood') {
window.myFoodList.remove(e.target.id);
}
}, false);
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With