I am trying to optimize the first chunk of code. Why doesn't my approach work? What's the best approach to shorten the code?

Question

The code I try to optimize:

function classToggle1() {
    document.getElementById("div1").classList.toggle('fa-folder');
    document.getElementById("div1").classList.toggle('fa-folder-open');
}
function classToggle2() {
    document.getElementById("div2").classList.toggle('fa-folder');
    document.getElementById("div2").classList.toggle('fa-folder-open');
}
function classToggle3() {
    document.getElementById("div3").classList.toggle('fa-folder');
    document.getElementById("div3").classList.toggle('fa-folder-open');
}
document.querySelector('#x').addEventListener('click', classToggle1);
document.querySelector('#y').addEventListener('click', classToggle2);
document.querySelector('#z').addEventListener('click', classToggle3);

My approach that doesn't work in browsers:

function classToggle(x) {
    document.getElementById(x).classList.toggle('fa-folder');
    document.getElementById(x).classList.toggle('fa-folder-open');
}
document.querySelector('#xx').addEventListener('click', classToggle("div1"));
document.querySelector('#yy').addEventListener('click', classToggle("div2"));
document.querySelector('#zz').addEventListener('click', classToggle("div3"));

Here I pass in "div1" "div2" and "div3" as arguments. My understanding is that those two code chunks should behave the same?

score 1 · Answer 1 · answered Jan 02 '22 at 08:58

In the first example you're passing in the function (classToggle1), and in the second you're invoking the function (which happens to return undefined). You can update the second to enlambda the call.

document.querySelector('#xx').addEventListener('click', () => classToggle("div1"));

Or

document.querySelector('#xx').addEventListener('click', function () { classToggle("div1"); })

score 1 · Accepted Answer · answered Jan 02 '22 at 08:58

This should work.

document.querySelector('#xx').addEventListener('click', () => classToggle("div1"));
document.querySelector('#yy').addEventListener('click', () => classToggle("div2"));
document.querySelector('#zz').addEventListener('click', () => classToggle("div3"));

The point is that, you are passing the output of the classToggle(x) instead of passing the function itself.

I am trying to optimize the first chunk of code. Why doesn't my approach work? What's the best approach to shorten the code?

2 Answers2