Search code examples
javascriptdomjslint

Getting warning don't make functions within a loop jslint

I'm Getting warning don't make functions within a loop on my code i'm using jslint my questions are

  1. is it ok to ignore this warning.
  2. how can i rectify this any alternative.

jsfiddle link

/*global prompt,alert,console,rgb*/
/*jslint plusplus: true */
/*jshint loopfunc: true */
var colors = [
    'rgb(255, 0, 0)',
    'rgb(255, 255, 0)',
    'rgb(255, 255, 255)',
    'rgb(0, 255, 255)',
    'rgb(0, 255, 0)',
    'rgb(0, 0, 255)'
];

var pickedColor = colors[2];
var square = document.querySelectorAll(".square");
var i;
var colorMatch = document.querySelector(".pickedColor");
for (i = 0; i < square.length; i++) {
    //add different color to square
    square[i].style.background = colors[i];
    //add eventlistner to square
    square[i].addEventListener('click', function () {
        'use strict';
        var selectedColor = (this.style.background);
        if (selectedColor === pickedColor) {
            console.log("True");
        } else {
            console.log('False');
        }
    });
}
colorMatch.textContent = pickedColor;
body{
    background: rgb(52, 73, 94);
}
h1{
    color: white;
}
.container{
    margin: 0 auto;
    max-width: 600px;
}
.square{
    width: 30%;
    margin: 1.66%;
    background-color: purple;
    padding-bottom: 30%;
    float: left;
}
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Color Game</title>
    <link rel="stylesheet" href="style.css">
</head>
<body>
   <h1>The Great <spam class="pickedColor"
   >RGB</spam> Color Game</h1>
   <div class="container">
       <div class="square"></div>
       <div class="square"></div>
       <div class="square"></div>
       <div class="square"></div>
       <div class="square"></div>
       <div class="square"></div>
   </div>
    <script src="script.js"></script>
</body>
</html>

enter image description here

In the meantime, thank you so much for your attention and participation.

Solution

  • In that specific case, you could ignore the warning, because your functions don't use any variables they close over (like i) that change in the course of the loop.

    But since they don't, there's no reason to create several copies of the function when just one will do:

    function squareClick() {
    'use strict';
    var selectedColor = (this.style.background);
    if (selectedColor === pickedColor) {
        console.log("True");
    } else {
        console.log('False');
    }
}

for (i = 0; i < square.length; i++) {
    //add different color to square
    square[i].style.background = colors[i];
    //add eventlistner to square
    square[i].addEventListener('click', squareClick);
}

    Where it would be a problem:

    Suppose you did this and there were 8 squares:

    for (i = 0; i < square.length; i++) {
    square[i].addEventListener('click', function() {
        alert(i); // <=== Using i
    });
}

    When you clicked those squares, you'd see the value 8, no matter which square you clicked, because those functions have a live reference to the i variable, not a copy of the variable as of when they were created.

    That's part of why jsLint warns you about this: Either you need to be careful (because you're using i), or it's pointless to create functions in the loop (because you aren't using anything that varies).

    Final note: In ES2015, you could safely do this:

    for (let n = 0; n < square.length; n++) {
    square[n].addEventListener('click', function() {
        alert(n); // <=== Using n
    });
}

    ...and you'd see 0, 1, 2, ... 7 depending on what square you clicked. This is because the ES2015+ semantics of the let declarations within for loops is specifically designed to ensure that each loop iteration gets its own copy of n. This is very different from the seemingly-near-identical version using var.

    Example with var; they all show 8:

    var square = document.querySelectorAll(".square");
for (var i = 0; i < square.length; i++) {
  square[i].addEventListener('click', function() {
    alert(i); // <=== Using i
  });
}
    <div class="square">zero</div>
<div class="square">one</div>
<div class="square">two</div>
<div class="square">three</div>
<div class="square">four</div>
<div class="square">five</div>
<div class="square">six</div>
<div class="square">seven</div>

    Example with let (on ES2015+ compliant browsers); they show 0...7:

    var square = document.querySelectorAll(".square");
for (let n = 0; n < square.length; n++) {
  square[n].addEventListener('click', function() {
    alert(n); // <=== Using n
  });
}
    <div class="square">zero</div>
<div class="square">one</div>
<div class="square">two</div>
<div class="square">three</div>
<div class="square">four</div>
<div class="square">five</div>
<div class="square">six</div>
<div class="square">seven</div>

    Note that the let has to be within the for for this treatment to kick in. That is,

    let n;
for (n = 0; n < square.length; ++n)

    is very different from

    for (let n = 0; n < square.length; ++n)

    The former would act like var in terms of functions created within the loop. The latter does not.