Optimize loop JavaScript

Question

ai up, look at this silly code. I want to pass either a string or an array and return the data. This code does work, but it sucks. What would you recommend as the best way to remove the duplication and optimizing it as much as possible? thanks :)

var getData = function (dataFile) {

  var ajaxResponse = [],
      loop,
      i;

  if(dataFile instanceof Array) {
    loop = dataFile.length;
    for(i = 0; i < loop; i++) {
      $.ajax({
        url: dataFile[i],
        type: "post",
        async: false,
        dataType: "json",
        success: function (data) {
          ajaxResponse[i] = data;
        }
      });
    }
  }
  else {
    $.ajax({
      url: dataFile,
      type: "post",
      async: false,
      dataType: "json",
      success: function (data) {
        ajaxResponse = data;
      }
    });
  }
  return ajaxResponse;
}

thanks people, I'll have to think about this. I could pass all the params as arrays if i wanted but that would cause me problems elsewhere. Basicaly i need to get the return values in the same way they came in, i.e. a single value or an array. The data being asked for is completely different. I could change it, but ill have to investigate which way is going to be better in the long run.

No need for a function if the code needs to be used only in one place. David Hedlund's answer does that :) — Patonza
– Patonza, Commented Dec 23, 2009 at 22:55
Somewhat OT, but it speaks to your optimization query: Don't make the queries synchronous if you can possibly avoid it! Synchronous ajax queries completely lock up the UI of some browsers -- not just your page, the entire browser. Even in the best case, your own page is completely locked up. Disable whatever you have to, but don't set async to false... — T.J. Crowder
– T.J. Crowder, Commented Dec 23, 2009 at 22:56

David Hedlund · Accepted Answer · 2009-12-23 23:21:54Z

8

If i'm understanding the code correctly, this should work:

if(!(dataFile instanceof Array))
    dataFile = [dataFile];

i.e., if it's not an array, create an array that contains only that one item. Then always do the loop, although in some cases it will have only one iteration.

I should add that you want to look into your use of the i variable in the for loop. You should change the declaration to for(var i..., rather than just for(i.... Introducing a variable without the var keyword will always make that variable global. This means that that loop is very likely to change the value of i in a completely different for loop, in another function, and you'll have absoutely no idea what's going on.

edited Dec 23, 2009 at 23:21

answered Dec 23, 2009 at 22:53

David Hedlund

130k33 gold badges205 silver badges223 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Annabelle Over a year ago

+1, but this does change the semantics of the function a bit. Now the function always returns an array whereas before it would return a single string when the input was a string.

David Hedlund Over a year ago

well i'd guess being able to rely on the type being returned is just as beneficial as being able to always use the variable as an array in this function. but if he really does want a single string returned in those cases, it'd be as simple as doing the same thing once more: return (dataFile instanceof Array) ? ajaxResponse : ajaxResponse[0];

user237899 · Accepted Answer · 2009-12-23 23:08:36Z

0

I like the elegance of David Hedlund's solution, and I think using only arrays will make life easier for you in the long run. But if that doesn't suit your fancy, try something like this (and please change my silly function names):

var getOneDatum = function (dataFile) {
  var ajaxResponse;
  $.ajax({
    url: dataFile,
    type: "post",
    async: false,
    dataType: "json",
    success: function (data) {
      ajaxResponse = data;
    }
  });
  return ajaxResponse;
}

var singleOrArray = function (fn, input) {
  var output;
  if (input instanceof Array) {
    var loop = input.length;
    output = [];
    for (i = 0; i < loop; i++) {
      output[i] = fn(input[i]);
    }
  } else {
    output = fn(input);
  }
  return output;
}

var getData = function (dataFile) {
  return singleOrArray(getOneDatum, dataFile);
}

The benefit here is that this singleOrArray function is very general, so you can reuse it any time you want to handle data that may or may not be an array.

answered Dec 23, 2009 at 23:08

user237899

1

2 Comments

Patonza Over a year ago

Nice and modular solution. I would too have coded that this way if I were using a language which didn't support closures :P

David Hedlund Over a year ago

arguably, with this approach, you could still have singleOrArray do if(!(input instanceof Array)) input = [input]; for(var i = 0; i < input.length; i++) fn(input[i]); and reduce some code.

user237899 · Accepted Answer · 2009-12-23 23:16:17Z

0

Good point, Patonza :)

var singleOrArray = function (fn, input) {
  var output;
  if (input instanceof Array) {
    var loop = input.length;
    output = [];
    for (i = 0; i < loop; i++) {
      output[i] = fn(input[i]);
    }
  } else {
    output = fn(input);
  }
  return output;
}

var getData = function (dataFile) {
  return singleOrArray(function (dataFile) {
    var ajaxResponse;
    $.ajax({
      url: dataFile,
      type: "post",
      async: false,
      dataType: "json",
      success: function (data) {
        ajaxResponse = data;
      }
    });
    return ajaxResponse;
  }, dataFile);
}

answered Dec 23, 2009 at 23:16

user237899

1

Comments

Alex · Accepted Answer · 2009-12-24 01:36:44Z

0

If you have a really large array that you are looping thorough you can use the following loop syntax to make it slightly faster (Be careful though as IMO it makes it much harder read!)

for (var i = dataFile.length; i--;) {
   // Do loop stuff
}

The reason this is slightly faster is the fact that the condition check and decrement are done in one operation as opposed to two.

answered Dec 24, 2009 at 1:36

Alex

37k11 gold badges57 silver badges69 bronze badges

Collectives™ on Stack Overflow

Optimize loop JavaScript

4 Answers 4

2 Comments

2 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related