Referencing an outer scoped variable on a loop within a loop

Question

I’m getting a JSHint/JSLint error on my code below.

Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. ($, currentVal)

This is directed at the $options.each loop where I’m pushing to currentVal.

Any thoughts on how to solve this?

$(".option input[type=radio]").change(function() {
  reload_filterstring(this);
});

$(".option input[type=checkbox]").change(function() {
  reload_filterstring(this);
});

function reload_filterstring(that) {
  var finalFilterQuery = {};

  var currentEl = "";

  (filterType = $this.attr("data-filter-type")),
  ($options = $this.find("ul.options input")),
  (query = $this.find('input[name="query"]')),
  (finalQuery = $('input[name="finalQuery"]')),
  (filterData = []),
  (filterQuery = null);

//  console.log(filterName);

currentVal = [];

$options.each(function() {
  if ($(this).prop("checked") == true) {
    currentVal.push($(this).attr("id"));
  }
});

finalFilterQuery[filterName] = currentVal.join("|");
  }

  // console.log(finalFilterQuery);
  var str = "";
  for (var key in finalFilterQuery) {
if (finalFilterQuery.hasOwnProperty(key)) {
  if (str != "") {
    str += ",";
  }
  str += key + "=" + finalFilterQuery[key];
}
  }
  console.log(str);
}

https://codepen.io/anon/pen/drMRem?editors=1111

It looks like you're missing the closing `}` for `reload_filterstring()` — mhatch, Mar 02 '19 at 22:52
Why are you using so many global variables? And assign to them in such a way with grouping and comma operators? Use variable declarations instead. — Bergi, Mar 02 '19 at 22:55
a) declare `$` as global b) use `Array.from($options).filter(el => el.checked).map(el => el.id).join("|");` — Bergi, Mar 02 '19 at 22:57
@Bergi Isn’t that still a function? I won’t be using arrow functions. — John the Painter, Mar 03 '19 at 11:56
@JohnthePainter Not sure what you mean "still a function"? And of course you can use normal function expressions if you don't like arrow functions for some weird reason. — Bergi, Mar 03 '19 at 12:46
Well arrow functions are a function so using them in the loop would still return the same error, no? — John the Painter, Mar 03 '19 at 12:48

score -1 · Answer 1 · answered Mar 02 '19 at 23:09

-1

I ran with codepen.io JS Analyse and the result was: Don't make functions within a loop. with a link to this JS Hint - don't make functions within a loop. You should move the function out instead of placing it inside a loop, but if you still insist on having a function inside the loop, you can narrow down the scope of currentVal inside the function like below:

$options.each(function($currentVal) {
  return function() {
    if ($(this).prop("checked") == true) {
      $currentVal.push($(this).attr("id"));
    };
  }(currentVal));

answered Mar 02 '19 at 23:09

Kei Huynh

29
5

Thanks for this. I’m unsure how my code will work correctly without the function (loop) within a loop. – John the Painter Mar 03 '19 at 11:55
"*Don't make functions within a loop.*" - uh, the OPs code doesn't do that? – Bergi Mar 03 '19 at 12:45

Referencing an outer scoped variable on a loop within a loop

1 Answers1