Using a Function with a Parameter inside a forEach Loop

Question

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.

addClass(active) : active is an undefined variable, therefore the parameter you pass to the function is undefined. Should be "active". — Jeremy Thille
– Jeremy Thille, Commented Oct 3, 2017 at 15:12
@JeremyThille I had actually tried putting quotation marks around active in the parameter parentheses and it hadn't worked. — cpsavante
– cpsavante, Commented Oct 3, 2017 at 15:34
BTW & is a bit-wise operator probably you have meant && for a short circuit operation. Besides why do you add the same class if it is already there..? — Redu
– Redu, Commented Oct 3, 2017 at 17:26

Tomalak · Accepted Answer · 2017-10-03 19:53:01Z

6

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.

edited Oct 3, 2017 at 19:53

answered Oct 3, 2017 at 15:19

Tomalak

339k68 gold badges547 silver badges635 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

cpsavante Over a year ago

Thank you for both the suggestion and recommendation. 'Code obfuscator' is quite the quip lol. I tried implementing your solution however, and I am getting the error message that addEventListener failed to execute on Event target: that 2 arguments are required, but only one is present. And I'm quite baffled as to what that means, as of right now

Tomalak Over a year ago

That's what I meant by my comment // where is the event listener??. Read the rest of my answer, too.

cpsavante Over a year ago

Woah, that's quite abit to wrap my head around, though I will definitely try my best to implement what I understand of this into my code. These are definitely some concepts that I haven't come across before. I'd like to ask through: You removed the if statements from your functions in the examples provided. Is this simply because in your implementation they aren't needed? Or that you just didn't add them simply because you didn't need to to share your point?

Tomalak Over a year ago

They are not needed. I trust classList.add and classList.remove to be smart enough to do a classList.contains check on their own. Doing the check myself would mean it ends up being done twice.

cpsavante Over a year ago

I implemented your third code snippet example and it worked: though I did have to exchange those commas for semicolons. Thank you again for your code and explanations. I feel that should I be able to have a firm grasp on the concepts you shared, I'll definitely be a better developer for it. Thanks again!

|

Andrzej Smyk · Accepted Answer · 2017-10-03 15:52:11Z

1

There are several issues with your code:

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.
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>

answered Oct 3, 2017 at 15:52

Andrzej Smyk

1,72411 silver badges21 bronze badges

1 Comment

cpsavante Over a year ago

Oh wow, I had definitely intended to use functional AND for sure: I would not have ever caught that error should you not have pointed it out to me. Thank you much!

Collectives™ on Stack Overflow

Using a Function with a Parameter inside a forEach Loop

2 Answers 2

6 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related