admin管理员组

文章数量:1287971

I'm currently using JsHint and am receiving warning W083: "Don't make functions within a loop". I read this post from JsLint Error Explanations and understand why you should not do this, which essentially boils down to the asychrnonous nature of JavaScript and the potential for variables to be overwritten.

However, I also read in a few other posts here on SO that although this is a faux pas it does not always lead to bugs depending on the situation.

My situation in particular that JsHint is plaining about is a for-loop that uses the jQuery $(selector).each() function within it. This function takes a function as a parameter. Below is a snippet of the code that I'm concerned about. Don't worry about what it actually does+ since I'm really just using this as an example:

for (var i = 0; i < sections.length; i++) {
    disableSectionState[sections[i].name] = {};

    $('.' + sections[i].name).each(function (index, element) {
        var id = $(element).attr('id');
        disableSectionState[sections[i].name][id] = $(element).attr('disabled');
    });

    if (sections[i].isChecked) {
        $('.' + sections[i].name).attr('disabled', 'disabled');
    }
}

Essentially, this is just a nested for-each loop within a for-loop, so I didn't think this would be too dangerous, but I'm obviously not familiar with all of the quirks in js. As of right now, everything is working properly with this function in particular, but I wanted to ask the munity about the dangers of this using jQuery's each function within a loop.

To prevent this being marked as a dupe I did see this SO question, but the only answer doesn't go into any detail or explain anything, and based on the ments it looks like an XY problem anyway. I'm more interested in the why this is when at it's core is that it's essentially a nested loop.

Is it really that much safer for this function to be extracted and named outside of the loop? If I copied the loop counter to a variable in scope of the anonymous function, would that eliminate the potential danger of this design? Is that function executed pletely asynchronously outside of the main for-loop?

+In case you're actually interested: This code is used to determine if certain fields should be disabled at page load if certain options are enabled.

I'm currently using JsHint and am receiving warning W083: "Don't make functions within a loop". I read this post from JsLint Error Explanations and understand why you should not do this, which essentially boils down to the asychrnonous nature of JavaScript and the potential for variables to be overwritten.

However, I also read in a few other posts here on SO that although this is a faux pas it does not always lead to bugs depending on the situation.

My situation in particular that JsHint is plaining about is a for-loop that uses the jQuery $(selector).each() function within it. This function takes a function as a parameter. Below is a snippet of the code that I'm concerned about. Don't worry about what it actually does+ since I'm really just using this as an example:

for (var i = 0; i < sections.length; i++) {
    disableSectionState[sections[i].name] = {};

    $('.' + sections[i].name).each(function (index, element) {
        var id = $(element).attr('id');
        disableSectionState[sections[i].name][id] = $(element).attr('disabled');
    });

    if (sections[i].isChecked) {
        $('.' + sections[i].name).attr('disabled', 'disabled');
    }
}

Essentially, this is just a nested for-each loop within a for-loop, so I didn't think this would be too dangerous, but I'm obviously not familiar with all of the quirks in js. As of right now, everything is working properly with this function in particular, but I wanted to ask the munity about the dangers of this using jQuery's each function within a loop.

To prevent this being marked as a dupe I did see this SO question, but the only answer doesn't go into any detail or explain anything, and based on the ments it looks like an XY problem anyway. I'm more interested in the why this is when at it's core is that it's essentially a nested loop.

Is it really that much safer for this function to be extracted and named outside of the loop? If I copied the loop counter to a variable in scope of the anonymous function, would that eliminate the potential danger of this design? Is that function executed pletely asynchronously outside of the main for-loop?

+In case you're actually interested: This code is used to determine if certain fields should be disabled at page load if certain options are enabled.

Share Improve this question edited May 23, 2017 at 12:15 CommunityBot 11 silver badge asked May 27, 2015 at 13:51 JNYRangerJNYRanger 7,09712 gold badges54 silver badges84 bronze badges
Add a ment  | 

2 Answers 2

Reset to default 12

The problem isn't using jQuery's each within the loop, it's repeatedly declaring a function. That can lead to some odd closure issues (the function closes on a reference to the loop counter, which still gets updated and can change) and can be a non-trivial performance problem on less clever VMs.

All JSHint is asking you to change is:

function doStuff(index, element) {
  var id = $(element).attr('id');
   disableSectionState[sections[i].name][id] = $(element).attr('disabled');
}

for (var i = 0; i < sections.length; i++) {
  disableSectionState[sections[i].name] = {};

  $('.' + sections[i].name).each(doStuff);

  if (sections[i].isChecked) {
    $('.' + sections[i].name).attr('disabled', 'disabled');
  }
}

Most of the dangers e when you're calling something asynchronously from within a loop and close over the loop counter. Take, for example:

for (var i = 0; i < urls.length; ++i) {
  $.ajax(urls[i], {success: function () {
    console.log(urls[i]);
  });
}

You may think it will log each URL as the requests succeed, but since i probably hit length before any requests have e back from the server, you're more likely to see the last URL repeatedly. It makes sense if you think about it, but can be a subtle bug if you aren't paying close attention to closure or have a more plex callback.

Not declaring functions within the loop forces you to explicitly bind or pass the loop counter, among other variables, and prevents this sort of thing from accidentally cropping up.

In some more naive implementations, the machine may also actually create a closure scope for the function every iteration of the loop, to avoid any potential oddities with variables that change within the loop. That can cause a lot of unnecessary scopes, which will have performance and memory implications.

JSHint is a very opinion-based syntax checker. It's kind of like deciding which type of citations to do on a paper MLA or APA. If you go with one, you just follow their rules because, most of the time, it is "right", but it's rarely ever wrong. JSHint also says to always use === but there may be cases to use == instead.

You can either follow the rules or ignore them with the following

// Code here will be linted with JSHint.
/* jshint ignore:start */
// Code here will be ignored by JSHint.
/* jshint ignore:end */

If you are going to use JSHint, I would just ply. It tends to keep the code a little more consistent, and when you start trying to work around one warning or error, it tends to start creating a bunch more

Is it really that much safer for this function to be extracted and named outside of the loop?

  • In practice, yes. In general, on case by case, maybe not.

If I copied the loop counter to a variable in scope of the anonymous function, would that eliminate the potential danger of this design?

  • No.

Is that function executed pletely asynchronously outside of the main for-loop?

  • Pretty sure it is.

本文标签: javascriptJsHint W083 Don39t Make Functions in Loop and jQuery39s each() functionStack Overflow