Reference "this" element outside of a for-loop in JavaScript (JSLint)

Question

Thank you in advance for your help!

I'm creating a simple tic-tac-toe game to help me learn native JavaScript, so my goal is to do as much as I can without jQuery or HTML/CSS for the game.

I am using a for-loop to create multiple div elements, adhering to JSLint. I am attaching an .addEventListener() as part of the process of creating my div elements to change the background color of that specific div when clicked.

I've been searching StackOverflow trying to use this to reference the specific div clicked. The only way I've been successful so far is by using an anonymous function within my for-loop. JSLint isn't impressed, and I get:

Don't make functions within a loop.

When I try to call an external function and pass this in, the entire div creation process just stops working and I'm not sure why.

What I have (that "works"): https://jsfiddle.net/typj2LLb/4/

// create game
var gameContainer = document.getElementById('board');
var createBoard = function() {
  'use strict';
  var index, square;
  for (index = 0; index < 9; index += 1) {
    square = document.createElement('div');
    square.className = 'tile';

    // tile event
    square.addEventListener('click', function() {
      this.style.backgroundColor = 'yellow';
    });

    gameContainer.appendChild(square);
  }
};

createBoard();

.tile {
  display: inline-block;
  height: 25vh;
  width: 30%;
  margin: 0 3px;
  border: 1px solid black;
}

<body>
  <div id="board"></div>
</body>

What I think I'm supposed to be doing (that doesn't work): https://jsfiddle.net/e4mstyy9/1/

// click-event
function changeColor(specificElement) {
  'use strict';
  specificElement.style.backgroundColor = 'yellow';
}

// create game
var gameContainer = document.getElementById('board');
var createBoard = function() {
  'use strict';
  var index, square;
  for (index = 0; index < 9; index += 1) {
    square = document.createElement('div');
    square.className = 'tile';

    // tile event
    square.addEventListener('click', changeColor(this));

    gameContainer.appendChild(square);
  }
};

createBoard();

.tile {
  display: inline-block;
  height: 25vh;
  width: 30%;
  margin: 0 3px;
  border: 1px solid black;
}

<body>
  <div id="board"></div>
</body>

Always include the relevant or your code in your comment. You can even use stack snippets instead of jsfiddle to create inline runnable examples. — Felix Kling, Oct 19 '16 at 20:02
Ah, thank you Felix. I'll make sure to do that in the future. — johndupper, Oct 19 '16 at 20:04

score 1 · Accepted Answer · edited May 23 '17 at 12:16

The advice to not create functions inside a loop is only relevant if you are referencing the loop variable (or something derived from that variable) inside the function (see JavaScript closure inside loops – simple practical example for why).

But since you are not doing this, your code is perfectly fine and you can keep using it.

Why does the second example not work?

When you have an expression such as foo(bar()), bar will be called first and its return value will be passed to foo.

In your code you have square.addEventListener('click', changeColor(this));. That means changeColor is executed first and its return value is passed to addEventListener.

However, the engine isn't even getting that far because executing changeColor(this) throws the error

Uncaught TypeError: Cannot read property 'style' of undefined

That's because the value you pass to changeColor (this) isn't a DOM element. Since you are using strict mode, the value of this is undefined, so you are executing changeColor(undefined).

One way to solve this without creating a function inside the loop body would be to simply pass changeColor instead of calling it (which requires us to use this again):

// click-event
function changeColor() {
  this.style.backgroundColor = 'yellow';
}

// ...

square.addEventListener('click', changeColor);

This is actually a good change because the function is only created once and reused, instead of creating a new event handler for every element.

We could just have `function changeColor() { this.style.backgroundColor = 'yellow'; }` and pass that like so: `square.addEventListener('click', changeColor));` https://jsfiddle.net/typj2LLb/7/ — lampyridae, Oct 19 '16 at 20:18
@lampyridae: Right, that makes more sense. I'll update the post. — Felix Kling, Oct 19 '16 at 20:19

Reference "this" element outside of a for-loop in JavaScript (JSLint)

1 Answers1