CODEWITHSUNDEEP
javascriptfor-loopbuttononclickopenlayers
Taras
Taras

Reputation: 276

How to make buttons execute the same code but with different variables accordingly?

dear all. Please, don't judge me hard, I am new in working with JavaScript and OpenLayers.

I am applying the same action of click functionality to multiple buttons that actually perform the same task but for different values.

I have six buttons that are placed in HTML section:

<button id="pan-to-kiev">Kiev</button>
<button id="pan-to-munich">Munich</button>
<button id="pan-to-vienna">Vienna</button>
<button id="pan-to-dresden">Dresden</button>
<button id="pan-to-enschede">Enschede</button>
<button id="pan-to-bonn">Bonn</button>

I also have the object that is placed in JS sheet: 

var cities = [
  {name: "Kiev", lonlat: [30.523400, 50.450100], color: '#006699', id:"pan-to-kiev"},
  {name: "Munich", lonlat: [11.581980, 48.135125], color: '#AB4450', id:"pan-to-munich"},
  {name: "Vienna", lonlat: [16.373819, 48.208174], color: '#93648D', id:"pan-to-vienna"},
  {name: "Dresden", lonlat: [13.737262, 51.050409], color: '#FFC65D', id:"pan-to-dresden"},
  {name: "Enschede", lonlat: [6.893662, 52.221537], color: '#F16745', id:"pan-to-enschede"},
  {name: "Bonn", lonlat: [7.0954900, 50.7343800], color: '#4CC3D9', id:"pan-to-bonn"}
];

This is a code that I wrote and which is executable, but I know that is bullshit since my code have to be more explicit and short because some variables repeat. 

function onClick(id, callback) {
  document.getElementById(id).addEventListener('click', callback);
}

onClick(cities[0]["id"], function() {
    view.animate({
      center: ol.proj.fromLonLat(cities[0]["lonlat"]),
      duration: 1500
    });
  });

onClick(cities[1]["id"], function() {
    view.animate({
      center: ol.proj.fromLonLat(cities[1]["lonlat"]),
      duration: 1500
    });
  });

onClick(cities[2]["id"], function() {
  view.animate({
    center: ol.proj.fromLonLat(cities[2]["lonlat"]),
    duration: 1500
  });
});

onClick(cities[3]["id"], function() {
  view.animate({
    center: ol.proj.fromLonLat(cities[3]["lonlat"]),
    duration: 1500
  });
});

onClick(cities[4]["id"], function() {
  view.animate({
    center: ol.proj.fromLonLat(cities[4]["lonlat"]),
    duration: 1500
  });
});

onClick(cities[5]["id"], function() {
  view.animate({
    center: ol.proj.fromLonLat(cities[5]["lonlat"]),
    duration: 1500
  });
});

And my idea does not work

 for (var city in cities) {
   onClick(cities[city]["id"], function() {
     view.animate({
       center: ol.proj.fromLonLat(cities[city]["lonlat"]),
       duration: 1500
     });
   });
   console.log(cities[city]["id"]);
}

Can you be so kind and help or at least explain to me what should I change here to be able to get better code with a for-loop?

Thank you so much, dear experts.

Upvotes: 1

Views: 565

Answers (2)

5ar
5ar

Reputation: 2210

In for (var city in cities) city is already an object, there is no point in using cities inside the loop, just use the object you have.

However, when iterating over an Array, you should be using for...of or even better forEach(), as suggested in the MDN documentation for for...in.

Here is the forEach implementation of the same loop with lambdas to avoid any problems with this:

cities.forEach(city => { 
  onClick(city.id, () => {
    view.animate({
      center: ol.proj.fromLonLat(city["lonlat"]),
      duration: 1500
    });
  });
  console.log(city.id); 
});

Upvotes: 1

Juorder Gonzalez
Juorder Gonzalez

Reputation: 1652

Your code is good for your purpose, the only problem is that your are loosing your context in the for loop, so you need to make a function closure to avoid this behavior, so you only need to change this part of your code to make it work.

for (var city in cities) {
   onClick(city.id, function(cityPassed){
     return function() {
       view.animate({
         center: ol.proj.fromLonLat(cityPassed["lonlat"]),
         duration: 1500
       });
     }
   // pass the city variable
   }(city));
}

as 5ar said, you don't need to access cities[city]["lonlat"] again because city is already the object inside the array, so you just need to use it, please check above Javascript Closures here: https://developer.mozilla.org/es/docs/Web/JavaScript/Closures

I hope it can help you.

Upvotes: 1

Related Questions