CODEWITHSUNDEEP
javascriptjqueryarraysobject
L-R
L-R

Reputation: 418

$.each loop of array is looping too many times

I have the following array and I'm trying to loop through it to create a div, within that div will be the image and a button that toggles the ingredients showing.

var cocktailsArray = [
  {image: "mojito.jpeg", ingredients:["lime", "rum", "sugar"]},
  {image: "pinacolada.jpeg", ingredients:["coconut liquer", "rum", "pineapple juice"]},
  {image: "tomcollins.jpeg", ingredients:["lemon", "gin", "sugar"]}
]

These are my functions:

var createObject = function(){
  $("div/div").addClass("cocktail").appendTo("main")
}

var createImage = function(image){
  var img = $("<img>").addClass("cocktail-image").appendTo(".cocktail");
  img.attr('src', image)
}

var createIngredients = function(ingredients){
  $("<button/>").text("Ingredients").addClass("ingredients-button").appendTo(".cocktail")
  $("div/div").addClass("ingredients").appendTo(".cocktail")
  $.each(ingredients, function(key, ingredient){
    var li = $("<li/>").text(ingredient).appendTo(".ingredients")
  })
}

var addCocktail = function(picture, ingredients){
  createObject();
  createImage(picture);
  createIngredients(ingredients);
}

and its called like this:

$(document).ready(function(){

  $.each(cocktailsArray, function(key, cocktail){
    addCocktail(cocktail.image, cocktail.ingredients)
  })

});

The output i'm getting is:

mojito pinacollada tomcollins
pinacollada tomcollins
tomcollins

Can anyone explain why the second two lines are populating? Why does it not just stop making objects once its looped through once?

Upvotes: 0

Views: 601

Answers (2)

Rory McCrossan
Rory McCrossan

Reputation: 337580

Firstly note that "div/div" is not a valid selector. If you want to create a div you can use $('<div /').

The issue is because you're appending to the new elements by classname within each iteration of the loop. This means that on successive iterations you fill the new elements, but also those .cocktail and .ingredient elements which were created previously.

To fix this you need to keep a reference to the created .cocktail and .ingredients within the loop, and then only append to them directly, something like this:

var cocktailsArray = [{
  image: "mojito.jpeg",
  ingredients: ["lime", "rum", "sugar"]
}, {
  image: "pinacolada.jpeg",
  ingredients: ["coconut liquer", "rum", "pineapple juice"]
}, {
  image: "tomcollins.jpeg",
  ingredients: ["lemon", "gin", "sugar"]
}]

var createObject = function() {
  return $("<div />").addClass("cocktail").appendTo("main")
}

var createImage = function($obj, image) {
  var img = $("<img>").addClass("cocktail-image").appendTo($obj);
  img.attr('src', image)
}

var createIngredients = function($obj, ingredients) {
  $("<button/>").text("Ingredients").addClass("ingredients-button").appendTo($obj)
  var $ings = $("<div />").addClass("ingredients").appendTo($obj)
  
  $.each(ingredients, function(key, ingredient) {
    var li = $("<li/>").text(ingredient).appendTo($ings)
  })
}

var addCocktail = function(picture, ingredients) {
  var $obj = createObject();
  createImage($obj, picture);
  createIngredients($obj, ingredients);
}

$(document).ready(function() {
  $.each(cocktailsArray, function(key, cocktail) {
    addCocktail(cocktail.image, cocktail.ingredients)
  })
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
<main></main>

Note how createObject() now returns the new div element, and that reference is used in the createImage() and createIngredients() functions. The same technique is used to store a reference to the .ingredients div, although that's kept local within createIngredients().

Upvotes: 1

Milind Anantwar
Milind Anantwar

Reputation: 82241

Well that is because you are appending content to all elements with class cocktail(using .appendTo()). you should be only appending to last element in matched set.

You can use :last selector in this case:

var img = $("<img>").addClass("cocktail-image").appendTo(".cocktail:last");

Upvotes: 2

Related Questions