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

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);
    }

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]);
      }
1 Like

A way to make the forEach more readable:

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

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]);
      }

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

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");
    }

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]);
          }

image

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");
    }
  }
}());

image

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]);
      }
    }
  }

image

@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");
    }

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]);
    }
  }

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.

image

  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

image

 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");
    }

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.

1 Like

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);
        }
      });
    }

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

1 Like

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?

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

1 Like

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]);
    }
  }

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

1 Like

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