Can this be written much more simplistically using a foreach?

Question

function GetDiagrams(componentID) {
    $.getJSON("../PlanView/GetDiagrams", { ComponentID: componentID }, function (diagrams) {
        for (var i = 0; i < diagrams.length; i++) {
            PaintDiagram(diagrams[i]);
        }
    });
}

All I am doing is calling PaintDiagram on each element returned. I tried Googling for a bit because I am pretty confident this is easily reducible, but was not able to pull up a quick example.

Is this function a candidate for any more refactoring?

What you have above would be the fastest, also i don't see any need to refactor a simple for loop. — aziz punjani, Feb 15 '12 at 17:05
Check out this question (and answers) http://stackoverflow.com/questions/242841/javascript-foreach-vs-for. — Matt K, Feb 15 '12 at 17:14

score 2 · Answer 1 · edited Feb 15 '12 at 17:29

2

Sure, I would definitely use the jquery $.each

 $.getJSON("../PlanView/GetDiagrams", { ComponentID: componentID }, function (diagrams) {
     $.each(diagrams, function() {
         PaintDiagram(this);
     });
 });

edited Feb 15 '12 at 17:29

gen_Eric

223,194
41
299
337

answered Feb 15 '12 at 17:05

Gabe

49,577
28
142
181

2

If you really want to go there, why not omit the args and say `PaintDiagram(this)`? (`each` binds each iteration's context to the object) – harpo Feb 15 '12 at 17:07
$.each is actually slower than for..in – SoWhat Feb 15 '12 at 17:18
Your syntax is slightly off. `$.each(diagrams, function(){ PaintDiagram(this); });`. I fixed it for you :-) – gen_Eric Feb 15 '12 at 17:28

score 0 · Answer 2 · answered Feb 15 '12 at 17:06

0

function GetDiagrams(componentID) {
    $.getJSON("../PlanView/GetDiagrams", { ComponentID: componentID }, function (diagrams) {
    for (i in diagrams) {
        PaintDiagram(diagrams[i]);
    }
    });
}

This is pretty much as good as you can do with js since it doesn't have a foreach

answered Feb 15 '12 at 17:06

SoWhat

5,564
2
28
59

1

@pete No, the i will be the key in the diagrams array – Michael Laffargue Feb 15 '12 at 17:15
1

I suggest checking `if(diagrams.hasOwnProperty(i))` to make sure you're not getting a property from the prototype chain. – gen_Eric Feb 15 '12 at 17:29

score 0 · Accepted Answer · answered Feb 15 '12 at 17:53

0

As per the comments, I opted to leave my solution as-is.

answered Feb 15 '12 at 17:53

Sean Anderson

27,963
30
126
237

Can this be written much more simplistically using a foreach?

3 Answers3