CODEWITHSUNDEEP
javascriptfunctionforeach
cpsavante
cpsavante

Reputation: 73

Using a Function with a Parameter inside a forEach Loop

In an attempt to make my code drier, I've decided to take an if/else statement that I use often and turn it into a function. I tried applying this function to a forEach loop, however, I am getting an error that the function's parameter is undefined.

Here's the code:

var addClass = (className) => {
    (this.classList.contains(className) === true)
        ? this.classList.remove('active') & this.classList.add(className)
        : this.classList.add(className);
}

exploreStory_buttons.forEach(el => el.addEventListener('mouseover'), addClass(active));

Any insight into what I'm doing wrong would be greatly appreciated. As per usual, I feel as though the solution may be simple: that I'm just not seeing it.

Upvotes: 3

Views: 319

Answers (2)

Tomalak
Tomalak

Reputation: 338248

You want to call the function in the proper context. There is Function#call for that purpose. You also need to use a normal function, as "fat arrow" functions handle this differently.

var addClass = function (className) {
    if (this.classList.contains(className)) {
        this.classList.remove('active');
        this.classList.add(className);
    } else {
        this.classList.add(className);
    }
}

exploreStory_buttons.forEach(el => {
    el.addEventListener('mouseover'); // where is the event listener??
    addClass.call(this, active);   // N.B. active is undefined here. Do you mean "active"?
});

You also do not want to abuse language features to write "smart" code. Use if/else and multiple lines for multiple statements, you are not a code obfuscator.

Assuming you want to add a "hover" behavior, the code would have look something like this:

function addClass(className) {
    return function () {
        this.classList.add(className);
    }
}
function removeClass(className) {
    return function () {
        this.classList.remove(className);
    }
}

var activate = addClass('active');
var deactivate = removeClass('active');

exploreStory_buttons.forEach(el => {
    el.addEventListener('mouseenter', activate);
    el.addEventListener('mouseleave', deactivate);
});

Note how addClass and removeClass return functions that can serve as event handlers. Storing them into activate and deactivate variables means you are not creating multiple equivalent copies of a function inside the loop, but in fact assign the same function to all elements.

The activate/deactivate variables and function-returning functions can be avoided by implementing a hover() function (jQuery has a similar feature):

function hover(elements, onenter, onleave) {
    elements.forEach(el => {
        el.addEventListener('mouseenter', onenter);
        el.addEventListener('mouseleave', onleave);
    });
}

hover(exploreStory_buttons, function () {
    this.addClass('active');
}, function () {
    this.removeClass('active');
});

Here the event handler functions are created only once and directly where they are needed. Much better than storing them in helper variables.

Upvotes: 6

Andrzej Smyk
Andrzej Smyk

Reputation: 1724

There are several issues with your code:

  1. I assume that on mouseover you would like to change the class of the element to active. In this case addClass function has to return another function, but regular one, not fat arrow, so this can be properly bound.

  2. Did you really wanted bitwise AND on this line:

    this.classList.remove('active') & this.classList.add(className)

or logical AND?

exploreStory_buttons = document.getElementsByClassName('explore-story');
var addClass = (className) => function() {
    this.classList.contains(className)
        ? (this.classList.remove('active') && this.classList.add(className))
        : this.classList.add(className);
}


Array.prototype.forEach.call(exploreStory_buttons, (el) => el.addEventListener('mouseover', addClass.call(el, 'active')));
.active {
  color: red
}
<button class="explore-story active">Story 1</button>
<button class="explore-story">Story 2</button>

Upvotes: 1

Related Questions