Where to put addEventListener so it doesn't fire multiple times

Question

I've got a Javascript file which draws a canvas with circles. If I click on a circle, the color must change. This works, but the eventHandler fires many times with one click. Because of this, the program lags a lot. How do I improve this?

var cvs = document.getElementById("canvas");

var ctx = cvs.getContext("2d");

class Circle {
  constructor(x, y, radius, color) {
    this.x = x;
    this.y = y;
    this.radius = radius;
    this.color = color;
  }

  draw = function() {
    ctx.beginPath();
    ctx.arc(this.x, this.y, this.radius, 0, 2 * Math.PI, false);
    ctx.strokeStyle = this.color;
    ctx.stroke();
  };

  update = function() {
    cvs.addEventListener("click", function(e) {
      var mousePosition = this.getMousePosition(e);
      this.checkForCollision(mousePosition);
    }.bind(this));
    this.draw();
  };

  getMousePosition = function(e) {
    var cRect = cvs.getBoundingClientRect();
    var canvasX = Math.round(e.clientX - cRect.left);
    var canvasY = Math.round(e.clientY - cRect.top);
    return {
      x: canvasX,
      y: canvasY
    }
  }

  checkForCollision = function(mPos) {
    if (mPos.x < this.x + this.radius && mPos.x > this.x - this.radius && mPos.y < this.y + this.radius && mPos.y > this.y - this.radius) {
      this.color = "blue";
    }
  }
}

var circles = [];
for (x = 0; x < 8; x++) {
  for (y = 0; y < 8; y++) {
    circles.push(new Circle(x * 100 + 30, y * 100 + 30, 20, "red"));
  }
}

function playGame() {
  requestAnimationFrame(playGame);
  ctx.clearRect(0, 0, 2000, 2000);
  circles.forEach(circle => circle.update());
}

playGame();

<canvas id="canvas"></canvas>

You are adding a new click event every draw cycle, which adds up quickly. Try moving the `addEventListener` outside of the draw/update cycle. — Reyno, Jul 27 '20 at 08:26
@TimMG Since you've declared an array of circles in `var circles = [];` what you could do instead is attach only one event listener for clicks, and when you detect a click, iterate through all Circle objects in the array to see if the mouse position at which the click occurred is within the current circle. — Septem151, Jul 27 '20 at 08:39

Septem151 · Accepted Answer · 2020-07-27T09:01:07.147

Currently, you are adding an event listener on every draw cycle because of where you're calling addEventListener. If you instead move addEventListener to the constructor of the circle, it will only be attached once per Circle object (in your case, 64 times):

var cvs = document.getElementById("canvas");

var ctx = cvs.getContext("2d");

class Circle {
    constructor(x, y, radius, color) {
        this.x = x;
        this.y = y;
        this.radius = radius;
        this.color = color;
        cvs.addEventListener("click", function (e) {
            var mousePosition = this.getMousePosition(e);
            this.checkForCollision(mousePosition);
            console.log('Click detected!');
        }.bind(this));
    }

    draw = function () {
        ctx.beginPath();
        ctx.arc(this.x, this.y, this.radius, 0, 2 * Math.PI, false);
        ctx.strokeStyle = this.color;
        ctx.stroke();
    };

    update = function () {
        this.draw();
    };

    getMousePosition = function (e) {
        var cRect = cvs.getBoundingClientRect();
        var canvasX = Math.round(e.clientX - cRect.left);
        var canvasY = Math.round(e.clientY - cRect.top);
        return {
            x: canvasX,
            y: canvasY
        }
    }

    checkForCollision = function (mPos) {
        if (mPos.x < this.x + this.radius && mPos.x > this.x - this.radius && mPos.y < this.y + this.radius && mPos.y > this.y - this.radius) {
            this.color = "blue";
        }
    }
}

var circles = [];
for(x = 0; x < 8; x++){
    for(y = 0; y < 8; y++){
        circles.push(new Circle(x * 100 + 30, y * 100 + 30, 20, "red"));
    }
}

function playGame() {
    requestAnimationFrame(playGame);
    ctx.clearRect(0, 0, 2000, 2000);
    circles.forEach(circle => circle.update());
}

playGame();

<canvas id="canvas" width="1000" height="1000"> </canvas>

If instead you only want to trigger the click event once per click, move the getMousePosition function outside of your Circle class and loop through the array of circles and checking if the click falls within their boundaries instead:

var cvs = document.getElementById("canvas");

var ctx = cvs.getContext("2d");

class Circle {
    constructor(x, y, radius, color) {
        this.x = x;
        this.y = y;
        this.radius = radius;
        this.color = color;
    }

    draw = function () {
        ctx.beginPath();
        ctx.arc(this.x, this.y, this.radius, 0, 2 * Math.PI, false);
        ctx.strokeStyle = this.color;
        ctx.stroke();
    };

    update = function () {
        this.draw();
    };

    checkForCollision = function(mPos) {
        if (mPos.x < this.x + this.radius && mPos.x > this.x - this.radius && mPos.y < this.y + this.radius && mPos.y > this.y - this.radius) {
            this.color = "blue";
            return true;
        }
        return false;
    }
}

var circles = [];
for(x = 0; x < 8; x++){
    for(y = 0; y < 8; y++){
        circles.push(new Circle(x * 100 + 30, y * 100 + 30, 20, "red"));
    }
}

function playGame() {
    requestAnimationFrame(playGame);
    ctx.clearRect(0, 0, 2000, 2000);
    circles.forEach(circle => circle.update());
}

function getMousePosition(e) {
    var cRect = cvs.getBoundingClientRect();
    var canvasX = Math.round(e.clientX - cRect.left);
    var canvasY = Math.round(e.clientY - cRect.top);
    return {
        x: canvasX,
        y: canvasY
    }
}

cvs.addEventListener("click", function (e) {
    console.log('Click detected!');
    var mousePosition = getMousePosition(e);
    circles.some(circle => {
        return circle.checkForCollision(mousePosition);
    });
});

playGame();

<canvas id="canvas" width="1000" height="1000"> </canvas>

score 0 · Answer 2 · answered Jul 27 '20 at 08:41

You are adding a new click event every draw cycle, which adds up quickly. Try moving the addEventListener outside of the draw/update cycle.

Also event bubbling can cause the function to fire multiple times. This can however easily be fixed with event.stopPropagation().

EventListener outside of the class.

var cvs = document.getElementById("canvas");

var ctx = cvs.getContext("2d");

class Circle {
    constructor(x, y, radius, color) {
        this.x = x;
        this.y = y;
        this.radius = radius;
        this.color = color;
    }

    draw = function () {
        ctx.beginPath();
        ctx.arc(this.x, this.y, this.radius, 0, 2 * Math.PI, false);
        ctx.strokeStyle = this.color;
        ctx.stroke();
    };

    update = function () {
        this.draw();
    };

    getMousePosition = function (e) {
        var cRect = cvs.getBoundingClientRect();
        var canvasX = Math.round(e.clientX - cRect.left);
        var canvasY = Math.round(e.clientY - cRect.top);
        return {
            x: canvasX,
            y: canvasY
        }
    }

    checkForCollision = function (mPos) {
        if (mPos.x < this.x + this.radius && mPos.x > this.x - this.radius && mPos.y < this.y + this.radius && mPos.y > this.y - this.radius) {
            this.color = "blue";
        }
    }
}

var circles = [];
for(x = 0; x < 8; x++){
    for(y = 0; y < 8; y++){
        circles.push(new Circle(x * 100 + 30, y * 100 + 30, 20, "red"));
    }
}

cvs.addEventListener("click", function (e) {
  e.stopPropagation();
  for(const circle of circles) {
    var mousePosition = circle.getMousePosition(e);
    circle.checkForCollision(mousePosition);
  }
});

function playGame() {
    requestAnimationFrame(playGame);
    ctx.clearRect(0, 0, 2000, 2000);
    
    for(const circle of circles) {
       circle.update();
    }
}

playGame();

canvas {
  width: 500px;
  height: 500px;
}

<canvas id="canvas"></canvas>

EventListener in constructor.

var cvs = document.getElementById("canvas");

var ctx = cvs.getContext("2d");

class Circle {
    constructor(x, y, radius, color) {
        this.x = x;
        this.y = y;
        this.radius = radius;
        this.color = color;
        
        cvs.addEventListener("click", function (e) {
            e.stopPropagation()
            var mousePosition = this.getMousePosition(e);
            this.checkForCollision(mousePosition);
        }.bind(this));
    }

    draw = function () {
        ctx.beginPath();
        ctx.arc(this.x, this.y, this.radius, 0, 2 * Math.PI, false);
        ctx.strokeStyle = this.color;
        ctx.stroke();
    };

    update = function () {
        this.draw();
    };

    getMousePosition = function (e) {
        var cRect = cvs.getBoundingClientRect();
        var canvasX = Math.round(e.clientX - cRect.left);
        var canvasY = Math.round(e.clientY - cRect.top);
        return {
            x: canvasX,
            y: canvasY
        }
    }

    checkForCollision = function (mPos) {
        if (mPos.x < this.x + this.radius && mPos.x > this.x - this.radius && mPos.y < this.y + this.radius && mPos.y > this.y - this.radius) {
            this.color = "blue";
        }
    }
}

var circles = [];
for(x = 0; x < 8; x++){
    for(y = 0; y < 8; y++){
        circles.push(new Circle(x * 100 + 30, y * 100 + 30, 20, "red"));
    }
}

function playGame() {
    requestAnimationFrame(playGame);
    ctx.clearRect(0, 0, 2000, 2000);
    
    for(const circle of circles) {
       circle.update();
    }
}

playGame();

canvas {
  width: 500px;
  height: 500px;
}

<canvas id="canvas"></canvas>

Why did you change circles.forEach(circle => circle.update()); to a for loop? — Tim MG, Jul 27 '20 at 08:50

Where to put addEventListener so it doesn't fire multiple times

2 Answers2