Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript DOM Scripting By Example Editing and Filtering Names Filter Invitees Who Have Not Responded

Pierrick Le Roy
Pierrick Le Roy
3,235 Points
Posted by Pierrick Le Roy
Pierrick Le Roy
3,235 Points

using filter() and forEach() instead of For Loops

Hello, I wanted to rewrite the code using higher order functions and finally managed to do it. We need to use the Array.from method to transform the HTML collections (li's) into a real array before being able to apply the filter and forEach methods.

Let me know if the code below can be improved.

Thank you, Pierrick

filterCheckBox.addEventListener('change', (e)=> {
    const isChecked = e.target.checked;
    const lis = ul.children;
        if(isChecked) {
            const arrayLis = Array.from(lis).filter(function(li) {
            return !(li.className==='responded');
            });
            arrayLis.forEach(function(li) {
            li.style.display = 'none';
            });
            } else {
            const arrayLis = Array.from(lis).forEach(function(li) {
            li.style.display = 'block';
            });   
        }
});
Rich Donnellan
Rich Donnellan
Treehouse Moderator 27,671 Points
Rich Donnellan
Rich Donnellan
Treehouse Moderator 27,671 Points

Hey, Pierrick! I edited your code block to show proper syntax highlighting. All you missed was the js part after the backticks in the code block.

4 Answers

Steven Parker
Steven Parker
229,732 Points
Steven Parker
Steven Parker
229,732 Points

Complication != improvement.

While I'm sure this was a fun exercise for it's own sake, I don't think I would call this code "improved". The methods used in the video are easier to read and comprehend, and therefore to maintain. And, features of for loops (like "break") that might be employed in future code changes are not available when using forEach. Plus I suspect that a benchmark comparison would show that the original code is also more efficient.

So, good job as far as a learning exercise, but I would not recommend this kind of refactoring for any real code.

Pierrick Le Roy
Pierrick Le Roy
3,235 Points
Pierrick Le Roy
Pierrick Le Roy
3,235 Points

Thank you for the feedback, Steven.

Pierrick Le Roy
Pierrick Le Roy
3,235 Points
Pierrick Le Roy
Pierrick Le Roy
3,235 Points

for the sake of my exercise, I rewrote it using a separate function at the top of the code (maybe I could use this function in some other parts of the app like sending email to the people who have not responded or to remove them in batch if there are hundreds of them):

const unResponded = function(li) {
        return !(li.className==='responded');
    }

The if...else statement might be more visible now by chaining the methods and removing unnecessary variables

filterCheckBox.addEventListener('change', (e)=> {
    const isChecked = e.target.checked;
    const lis = ul.children;
       if(isChecked) {
              Array.from(lis).filter(unResponded).forEach(function(li) {
              li.style.display = 'none';
              });
        } else {
              Array.from(lis).forEach(function(li) {
              li.style.display = 'block';
              });
        }
});
Steven Parker
Steven Parker
229,732 Points
Steven Parker
Steven Parker
229,732 Points

For further simplification, instead of negating an equality comparison :point_right: !(li.className==='responded')
... you could just use an inequality comparison :point_right: li.className !== 'responded'

But I should point out that className may contain a space-separated collection of classes, so a simple comparison might not be the best way to make this test. To avoid problems when other classes are present you might want to do this instead:

!li.classList.contains('responded')
Pierrick Le Roy
Pierrick Le Roy
3,235 Points
Pierrick Le Roy
Pierrick Le Roy
3,235 Points

Great! Thanks for the tips!

clickpass
clickpass
23,992 Points
clickpass
clickpass
23,992 Points

I started out using filter, but realized it was a little easier to use a forEach with a ternary:

filterCheckbox.addEventListener('change', (e) => {
    const isChecked = e.target.checked;
    const lis = ul.children;
    if (isChecked){
        Array.from(lis).forEach((li)=>{
           li.className === 'responded' ? li.style.display = 'block' : li.style.display = 'none';
        });

    } else{
        Array.from(lis).forEach((li)=>{
           li.style.display = 'block';
        });
    }
});
Steven Parker
Steven Parker
229,732 Points
Steven Parker
Steven Parker
229,732 Points

That may work, but it's not the "best practice" way to use a ternary. Generally, you'd want to make the assignment using the ternary as the right-side expression, like this:

           li.style.display = li.className==='responded' ? 'block' : 'none';

And instead of converting the HTMLcollection to an array and then using "forEach", this might be a good place for a compact and efficient "for...of":

        for (let li of lis) {
           li.style.display = li.className==='responded' ? 'block' : 'none';
        }
clickpass
clickpass
23,992 Points

Steven thanks for the pointers. Your suggestions certainly are much appreciated. I do like how clean your code is, but the tone of your comments is somewhat nasty. Are you angry?

Steven Parker
Steven Parker
229,732 Points
Steven Parker
Steven Parker
229,732 Points

Rafael, I'm not sure what you are interpreting as "tone". My intent is to be helpful, and I try to make my comments concise, factual, and to-the-point. Did I say something that seemed personal and/or negative?