0
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?

Sean Anderson
  • 26,361
  • 27
  • 120
  • 228

3 Answers3

2

Sure, I would definitely use the jquery $.each

 $.getJSON("../PlanView/GetDiagrams", { ComponentID: componentID }, function (diagrams) {
     $.each(diagrams, function() {
         PaintDiagram(this);
     });
 });
gen_Eric
  • 214,658
  • 40
  • 293
  • 332
Gabe
  • 48,376
  • 28
  • 140
  • 179
  • 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
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

SoWhat
  • 5,338
  • 2
  • 27
  • 57
0

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

Sean Anderson
  • 26,361
  • 27
  • 120
  • 228