CODEWITHSUNDEEP
javascriptperformance
mattomatto
mattomatto

Reputation: 5

Is it more efficient to use a for loop?

Here's an example of some code I have on my website... This actually goes on for another 12 iterations. Is it possible to use a for loop to do this? Is there a more efficient way? Thanks!

document.getElementById("1").onclick=function(){
    lyre[0]=Number(!lyre[0]);
    document.getElementById("1").innerHTML=String(lyre[0]);
}
    document.getElementById("2").onclick=function(){
    lyre[1]=Number(!lyre[1]);
    document.getElementById("2").innerHTML=String(lyre[1]);
}
document.getElementById("3").onclick=function(){
    lyre[2]=Number(!lyre[2]);
    document.getElementById("3").innerHTML=String(lyre[2]);
}
 document.getElementById("4").onclick=function(){
    lyre[3]=Number(!lyre[3]);
    document.getElementById("4").innerHTML=String(lyre[3]);
}

Upvotes: 0

Views: 115

Answers (3)

Nick Husher
Nick Husher

Reputation: 1884

At its most basic, you can do something like this:

function attachListener(lyre, lyreIndex, id) {

  document.getElementById(id).onclick = function() {
    // assign zero or one based on the current value:
    lyre[lyreIndex] = !lyre[lyreIndex] ? 0 : 1;

    // update the display of the value:
    document.getElementById(id).innerHTML = lyre[lyreIndex];
  }
}

for(var i = 0; i < lyre.length; i += 1) {
  attachListener(lyre, i, i + 1);
}

I've modified the code to be more idiomatically javascript. In general, you should avoid the primitive constructors (e.g. Number and String) and use the literal versions of the same.

There are other ways to make this more maintainable and idiomatic, but this should serve your need.

If you're worried about performance, a few dozen event listeners isn't usually something to be concerned with. If you are dealing with more than that, though, you should consider delegating your events. This limits the number of discrete event listeners while maintaining the same functionality:

document.getElementById('parent').onclick = function(e) {
  var node = (e.target || e.srcElement),
      index = node.id;

  if(lyre[index] === undefined) { return; }

  lyre[index] = !lyre[index] ? 1 : 0;

  node.innerHTML = lyre[index];
};

Where parent is the ID of some node in the document that all the elements share. Here's a working JSBin that illustrates the concept.

Upvotes: 2

Learner
Learner

Reputation: 3396

Whenever I would spot a pattern, I'd create first of all a function. Then I would use a "for" (see the other answers how)... This will make my code well structured and easy to maintain.

But, as others said, it won't be any better in terms of performance.

Upvotes: 0

Guilherme Sehn
Guilherme Sehn

Reputation: 6787

Yes, it's possible to use for to accomplish what you are trying to do. Although it won't be any better in terms of performance as you will execute a little bit of more operations, it's much better in terms of code organization. In most of the cases performance difference won't be perceptible, so I really suggest you use a loop for this.

function setEvent(index) {
    var id = index + 1;
    document.getElementById(id).onclick=function(){
        lyre[index]=Number(!lyre[index]);
        document.getElementById(id).innerHTML=String(lyre[index]);
    }
}

for (var i = 0, j = lyre.length; i < j; i++) {
    setEvent(i);
}

Upvotes: 1

Related Questions