How would I write these codes using for Loop instead of forEach?

asasass · November 21, 2018, 8:49am

I just want to see how it would be written.

Code 1.)

    function pauseAllButtons() {
      const buttons = document.querySelectorAll(".playButton");
      buttons.forEach(function hidePause(button) {
        if (isPlaying(button)) {
          showPlayButton(button);
        }
      });
    }

Code 2.)

    function hideAllButtons(button) {
      button.querySelectorAll(".play, .pause, .speaker").forEach(hide);
    }

Paul_Wilkins · November 21, 2018, 9:58am

Here’s the forEach code:

      buttons.forEach(function hidePause(button) {
        if (isPlaying(button)) {
          showPlayButton(button);
        }
      });

and here is the for loop version of that code.

      for (let i = 0; i < buttons.length; i += 1) {
        if (isPlaying(buttons[i])) {
          showPlayButton(buttons[i]);
        }
      }

Here’s the forEach version of the other code:

      button.querySelectorAll(".play, .pause, .speaker").forEach(hide);

And here’s the for loop version:

      const buttons = button.querySelectorAll(".play, .pause, .speaker");
      for (let i = 0; i < buttons.length; i += 1) {
        hide(buttons[i]);
      }

Dormilich · November 21, 2018, 10:04am

A way to make the forEach more readable:

const buttons = document.querySelectorAll(".playButton")
Array.from(buttons)
     .filter(isPlaying)
     .forEach(showPlayButton)

asasass · November 21, 2018, 1:44pm

I tried putting those in here and it doesn’t work:

Working version: forEach

    function pauseAllButtons() {
      const buttons = document.querySelectorAll(".playButton");
      buttons.forEach(function hidePause(button) {
        if (isPlaying(button)) {
          showPlayButton(button);
        }
      });
    }

    function showPauseButton(button) {
      const pause = getPause(button);
      pauseAllButtons();
      hideAllButtons(button);
      show(pause);
      button.classList.add("active");
    }

Code not working here: for Loop
Console isn’t telling me anything.
I should’ve provided a jsfiddle with it, I didn’t know additional code would be needed.

    function pauseAllButtons() {
      for (let i = 0; i < buttons.length; i += 1) {
        if (isPlaying(buttons[i])) {
          showPlayButton(buttons[i]);
        }
      }
    }

    function hideAllButtons(button) {
      const buttons = button.querySelectorAll(".play, .pause, .speaker");
      for (let i = 0; i < buttons.length; i += 1) {
        hide(buttons[i]);
      }

Dormilich · November 21, 2018, 2:19pm

you forgot to define the buttons variable in pauseAllButtons().

asasass · November 21, 2018, 2:24pm

It was never defined here, or needed to be, or was it?


    function pauseAllButtons() {
      const buttons = document.querySelectorAll(".playButton");
      buttons.forEach(function hidePause(button) {
        if (isPlaying(button)) {
          showPlayButton(button);
        }
      });
    }

    function showPauseButton(button) {
      const pause = getPause(button);
      pauseAllButtons();
      hideAllButtons(button);
      show(pause);
      button.classList.add("active");
    }

asasass · November 21, 2018, 4:35pm

I’m getting an error message:
How would this be fixed?

      function hideAllButtons(button) {
          const buttons = button.querySelectorAll(".play, .pause, .speaker");
          for (let i = 0; i < buttons.length; i += 1) {
              hide(buttons[i]);
          }

asasass · November 21, 2018, 9:29pm

This removes the unexpected ‘for’

But unexpected ‘let’ is still appearing.

Maybe those errors, warnings can be ignored.

It still doesn’t work though:

Maybe I have the braces in the wrong spot:

Too many, too few.

Usually console tells you stuff, which helps you to figure out how to fix the issue, but in this case it’s not telling me anything.

    function hideAllButtons(button) {
        const buttons = button.querySelectorAll(".play, .pause, .speaker");
        for (let i = 0; i < buttons.length; i += 1) {
            hide(buttons[i]);
        }

        function pauseAllButtons() {
            for (let i = 0; i < buttons.length; i += 1) {
                if (isPlaying(buttons[i])) {
                    showPlayButton(buttons[i]);
                }
            }
        }

        function initButton(selector) {
            const wrapper = document.querySelector(selector);
            wrapper.addEventListener("click", playButtonClickHandler);
        }
        initButton(".wrapa");
    }
}());

Or should the ending braces be set like this:

  function hideAllButtons(button) {
    const buttons = button.querySelectorAll(".play, .pause, .speaker");
    for (let i = 0; i < buttons.length; i += 1) {
      hide(buttons[i]);
    }

    function pauseAllButtons() {
      for (let i = 0; i < buttons.length; i += 1) {
        if (isPlaying(buttons[i])) {
          showPlayButton(buttons[i]);
        }

      }

      function initButton(selector) {
        const wrapper = document.querySelector(selector);
        wrapper.addEventListener("click", playButtonClickHandler);
      }
      initButton(".wrapa");
    }
  }
}());

asasass · November 21, 2018, 9:50pm

Code

This one gets 2 end braces:

  function hideAllButtons(button) {
    const buttons = button.querySelectorAll(".play, .pause, .speaker");
    for (let i = 0; i < buttons.length; i += 1) {
      hide(buttons[i]);
    }
  }

This one gets 3 end braces:
But then how come “buttons” is colored in gray, and not blue, when they should be blue.

  function pauseAllButtons() {
    for (let i = 0; i < buttons.length; i += 1) {
      if (isPlaying(buttons[i])) {
        showPlayButton(buttons[i]);
      }
    }
  }

asasass · November 21, 2018, 11:20pm

@Paul_Wilkins

I think I got it working here, but is this set correctly?

I guess when using “for loop,” buttons, and button is required to be filled in

function pauseAllButtons(buttons) {
pauseAllButtons(button);

 function pauseAllButtons(buttons) {
    for (let i = 0; i < buttons.length; i += 1) {
      if (isPlaying(buttons[i])) {
        showPlayButton(buttons[i]);
      }
    }

  }

  function showPauseButton(button) {
    const pause = getPause(button);
    pauseAllButtons(button);
    hideAllButtons(button);
    show(pause);
    button.classList.add("active");
  }

When using forEach, those were able to be left empty:

function pauseAllButtons() {
pauseAllButtons();

forEach

    function pauseAllButtons() {
      const buttons = document.querySelectorAll(".playButton");
      buttons.forEach(function hidePause(button) {
        if (isPlaying(button)) {
          showPlayButton(button);
        }
      });
    }

    function showPauseButton(button) {
      const pause = getPause(button);
      pauseAllButtons();
      hideAllButtons(button);
      show(pause);
      button.classList.add("active");
    }

asasass · November 21, 2018, 11:50pm

How would I have added “for loop” to this one which is condensed?

  function hideAllButtons(button) {
    const buttonSelectors = ".play, .pause, .speaker";
    button.querySelectorAll(buttonSelectors).forEach(hide);
  }

This is how the other one was done:


  function hideAllButtons(button) {
    const buttons = button.querySelectorAll(".play, .pause, .speaker");
    for (let i = 0; i < buttons.length; i += 1) {
      hide(buttons[i]);
    }
  }

asasass · November 22, 2018, 2:00am

This one uses For Loop
What was done wrong with this one?
Is that something that can be easily fixed?

After you click on another button they don’t change back to the play button, and instead they stay on pause. The audio pauses without an issue, it’s just the buttons that don’t change back for some reason.

  function hideAllButtons(button) {
    const buttons = button.querySelectorAll(".play, .pause, .speaker");
    for (let i = 0; i < buttons.length; i += 1) {
      hide(buttons[i]);
    }
  }

  function pauseAllButtons(buttons) {
    for (let i = 0; i < buttons.length; i += 1) {
      if (isPlaying(buttons[i])) {
        showPlayButton(buttons[i]);
      }
    }

  }

    function showPauseButton(button) {
      const pause = getPause(button);
      pauseAllButtons(button);
      hideAllButtons(button);
      show(pause);
      button.classList.add("active");
    }

Looking at how this one was set up, are you able to determine what I would change in the above code to fix that issue?

This one uses forEach

 function hideAllButtons(button) {
      button.querySelectorAll(".play, .pause, .speaker").forEach(hide);
    }

   function pauseAllButtons() {
      const buttons = document.querySelectorAll(".playButton");
      buttons.forEach(function hidePause(button) {
        if (isPlaying(button)) {
          showPlayButton(button);
        }
      });
    }

    function showPauseButton(button) {
      const pause = getPause(button);
      pauseAllButtons();
      hideAllButtons(button);
      show(pause);
      button.classList.add("active");
    }

Paul_Wilkins · November 22, 2018, 3:53am

You fix it by not using for loops. For loops banned by jslint because they can result in far too much confusion.

Use forEach loops instead, which results in better structured and more easily understood code.

asasass · November 22, 2018, 3:56am

What’s the difference between how both of these codes are written?

This one uses Array.from and .filter.

This one also doesn’t require:
(function hidePause(button)
And how come it’s no longer needed here?

    function pauseAllButtons() {
      const buttons = document.querySelectorAll(".playButton")
      Array.from(buttons)
        .filter(isPlaying)
        .forEach(showPlayButton)
    }

This one uses if
And requires this line:
(function hidePause(button)


    function pauseAllButtons() {
      const buttons = document.querySelectorAll(".playButton");
      buttons.forEach(function hidePause(button) {
        if (isPlaying(button)) {
          showPlayButton(button);
        }
      });
    }

Paul_Wilkins · November 22, 2018, 3:58am

The code that uses filter is easier to understand. That’s the main difference.

asasass · November 22, 2018, 3:17pm

I was told it’s better to use this:

    function hideAllButtons() {
      let btnArray =
        Array.prototype.slice.call(document.querySelectorAll(".play, .pause, .speaker"));
      btnArray.forEach(hide)
    }

Than this:

    function hideAllButtons(button) {
      button.querySelectorAll(".play, .pause, .speaker").forEach(hide);
    }

Do you agree?

Dormilich · November 22, 2018, 3:21pm

Only if you use old browsers, as NodeList.forEach is a pretty recent addition.

asasass · November 22, 2018, 4:32pm

This:

      function hideAllButtons(button) {
        const buttonSelectors = ".play, .pause, .speaker";
        button.querySelectorAll(buttonSelectors).forEach(hide);
      }

It would’ve been changed to this, I just got it:

  function hideAllButtons(button) {
    const buttonSelectors = ".play, .pause, .speaker";
    const buttons = button.querySelectorAll(buttonSelectors);
    for (let i = 0; i < buttons.length; i += 1) {
      hide(buttons[i]);
    }
  }

Dormilich · November 22, 2018, 4:40pm

I’d prefer the first version. much more readable than the clunky for loop.

asasass · November 22, 2018, 4:41pm

I like to see how things can be written different ways, using different things.
But yes, forEach is much more readable.

Topic		Replies	Views
Converting to for loop JavaScript	15	867	November 27, 2019
Converting for loop to foreach loop JavaScript	7	14436	April 18, 2017
JavaScript for loop question JavaScript	3	750	October 8, 2014
How to make my Javascript code even more better & cleaner? JavaScript	6	2325	October 20, 2017
Stop all previous players that are not being listened to JavaScript	3	521	March 5, 2019

Related topics