0
\$\begingroup\$

I created this Javascript function to manage six simple audio players within a page, so that when one of them is put into play, it shows the pause button, and any other player that was playing previously, shows the play button.

Sometimes the play / pause mechanism gets stuck (although I don't know if it depends on too many statements).

And also JS Fiddle reports "this function has too many statements (78)". Would there be a way to reduce the statements?

        window.onload = function() {

      // PLAY 1

      var myAudio = document.getElementById('my-audio');
      var suona = document.getElementById('suona');
      var pause = document.getElementById('pause');
      var bar = document.getElementById('bar');
      var progress = document.getElementById('progress');

      function displayControls() {
        suona.style.display = "block";
      }

      // check that the media is ready before displaying the controls
      if (myAudio.paused) {
        displayControls();
      } else {
        // not ready yet - wait for canplay event
        myAudio.addEventListener('canplay', function() {
          displayControls();
        });
      }

      suona.addEventListener('click', function() {
        myAudio.play();
        suona.style.display = "none";
        pause.style.display = "block";

        pause2.style.display = "none";
        suona2.style.display = "block";
        pause3.style.display = "none";
        suona3.style.display = "block";
        pause4.style.display = "none";
        suona4.style.display = "block";
        pause5.style.display = "none";
        suona5.style.display = "block";
        pause6.style.display = "none";
        suona6.style.display = "block";
      });

      pause.addEventListener('click', function() {
        myAudio.pause();
        pause.style.display = "none";
        suona.style.display = "block";
      });

      // display progress

      myAudio.addEventListener('timeupdate', function() {
        //sets the percentage
        bar.style.width = parseInt(((myAudio.currentTime / myAudio.duration) * 100), 10) + "%";
      });

      progress.addEventListener('click', function(e) {

        // calculate the normalized position clicked
        var clickPosition = (e.pageX - this.offsetLeft) / this.offsetWidth;
        var clickTime = clickPosition * myAudio.duration;

        // move the playhead to the correct position
        myAudio.currentTime = clickTime;
        myBufferedTimeRanges = myAudio.buffered;
        mySeekableTimeRanges = myAudio.seekable;
      });

      // PLAY 2

      var myAudio2 = document.getElementById('my-audio2');
      var suona2 = document.getElementById('suona2');
      var pause2 = document.getElementById('pause2');
      var bar2 = document.getElementById('bar2');
      var progress2 = document.getElementById('progress2');


      function displayControls2() {
        suona2.style.display = "block";
      }

      // check that the media is ready before displaying the controls
      if (myAudio2.paused) {
        displayControls2();
      } else {
        // not ready yet - wait for canplay event
        myAudio2.addEventListener('canplay', function() {
          displayControls2();
        });
      }

      suona2.addEventListener('click', function() {
        myAudio2.play();
        suona2.style.display = "none";
        pause2.style.display = "block";

        pause.style.display = "none";
        suona.style.display = "block";
        pause3.style.display = "none";
        suona3.style.display = "block";
        pause4.style.display = "none";
        suona4.style.display = "block";
        pause5.style.display = "none";
        suona5.style.display = "block";
        pause6.style.display = "none";
        suona6.style.display = "block";
      });

      pause2.addEventListener('click', function() {
        myAudio2.pause();
        pause2.style.display = "none";
        suona2.style.display = "block";
      });

      // display progress

      myAudio2.addEventListener('timeupdate', function() {
        //sets the percentage
        bar2.style.width = parseInt(((myAudio2.currentTime / myAudio2.duration) * 100), 10) + "%";
      });

      progress2.addEventListener('click', function(e) {

        // calculate the normalized position clicked
        var clickPosition = (e.pageX - this.offsetLeft) / this.offsetWidth;
        var clickTime = clickPosition * myAudio2.duration;

        // move the playhead to the correct position
        myAudio2.currentTime = clickTime;
        myBufferedTimeRanges = myAudio2.buffered;
        mySeekableTimeRanges = myAudio2.seekable;
      });

      // PLAY 3

      var myAudio3 = document.getElementById('my-audio3');
      var suona3 = document.getElementById('suona3');
      var pause3 = document.getElementById('pause3');
      var bar3 = document.getElementById('bar3');
      var progress3 = document.getElementById('progress3');

      function displayControls3() {
        suona3.style.display = "block";
      }

      // check that the media is ready before displaying the controls
      if (myAudio3.paused) {
        displayControls3();
      } else {
        // not ready yet - wait for canplay event
        myAudio3.addEventListener('canplay', function() {
          displayControls3();
        });
      }

      suona3.addEventListener('click', function() {
        myAudio3.play();
        suona3.style.display = "none";
        pause3.style.display = "block";

        pause.style.display = "none";
        suona.style.display = "block";
        pause2.style.display = "none";
        suona2.style.display = "block";
        pause4.style.display = "none";
        suona4.style.display = "block";
        pause5.style.display = "none";
        suona5.style.display = "block";
        pause6.style.display = "none";
        suona6.style.display = "block";
      });

      pause3.addEventListener('click', function() {
        myAudio3.pause();
        pause3.style.display = "none";
        suona3.style.display = "block";
      });

      // display progress

      myAudio3.addEventListener('timeupdate', function() {
        //sets the percentage
        bar3.style.width = parseInt(((myAudio3.currentTime / myAudio3.duration) * 100), 10) + "%";
      });

      progress3.addEventListener('click', function(e) {

        // calculate the normalized position clicked
        var clickPosition = (e.pageX - this.offsetLeft) / this.offsetWidth;
        var clickTime = clickPosition * myAudio3.duration;

        // move the playhead to the correct position
        myAudio3.currentTime = clickTime;
        myBufferedTimeRanges = myAudio3.buffered;
        mySeekableTimeRanges = myAudio3.seekable;
      });

      // PLAY 4

      var myAudio4 = document.getElementById('my-audio4');
      var suona4 = document.getElementById('suona4');
      var pause4 = document.getElementById('pause4');
      var bar4 = document.getElementById('bar4');
      var progress4 = document.getElementById('progress4');

      function displayControls4() {
        suona4.style.display = "block";
      }

      // check that the media is ready before displaying the controls
      if (myAudio4.paused) {
        displayControls4();
      } else {
        // not ready yet - wait for canplay event
        myAudio4.addEventListener('canplay', function() {
          displayControls4();
        });
      }

      suona4.addEventListener('click', function() {
        myAudio4.play();
        suona4.style.display = "none";
        pause4.style.display = "block";

        pause.style.display = "none";
        suona.style.display = "block";
        pause2.style.display = "none";
        suona2.style.display = "block";
        pause3.style.display = "none";
        suona3.style.display = "block";
        pause5.style.display = "none";
        suona5.style.display = "block";
        pause6.style.display = "none";
        suona6.style.display = "block";
      });

      pause4.addEventListener('click', function() {
        myAudio4.pause();
        pause4.style.display = "none";
        suona4.style.display = "block";
      });

      // display progress

      myAudio4.addEventListener('timeupdate', function() {
        //sets the percentage
        bar4.style.width = parseInt(((myAudio4.currentTime / myAudio4.duration) * 100), 10) + "%";
      });

      progress4.addEventListener('click', function(e) {

        // calculate the normalized position clicked
        var clickPosition = (e.pageX - this.offsetLeft) / this.offsetWidth;
        var clickTime = clickPosition * myAudio4.duration;

        // move the playhead to the correct position
        myAudio4.currentTime = clickTime;
        myBufferedTimeRanges = myAudio4.buffered;
        mySeekableTimeRanges = myAudio4.seekable;
      });

      // PLAY 5

      var myAudio5 = document.getElementById('my-audio5');
      var suona5 = document.getElementById('suona5');
      var pause5 = document.getElementById('pause5');
      var bar5 = document.getElementById('bar5');
      var progress5 = document.getElementById('progress5');


      function displayControls5() {
        suona5.style.display = "block";
      }

      // check that the media is ready before displaying the controls
      if (myAudio5.paused) {
        displayControls5();
      } else {
        // not ready yet - wait for canplay event
        myAudio5.addEventListener('canplay', function() {
          displayControls5();
        });
      }

      suona5.addEventListener('click', function() {
        myAudio5.play();
        suona5.style.display = "none";
        pause5.style.display = "block";

        pause.style.display = "none";
        suona.style.display = "block";
        pause2.style.display = "none";
        suona2.style.display = "block";
        pause3.style.display = "none";
        suona3.style.display = "block";
        pause4.style.display = "none";
        suona4.style.display = "block";
        pause6.style.display = "none";
        suona6.style.display = "block";
      });

      pause5.addEventListener('click', function() {
        myAudio5.pause();
        pause5.style.display = "none";
        suona5.style.display = "block";
      });

      // display progress

      myAudio5.addEventListener('timeupdate', function() {
        //sets the percentage
        bar5.style.width = parseInt(((myAudio5.currentTime / myAudio5.duration) * 100), 10) + "%";
      });

      progress5.addEventListener('click', function(e) {

        // calculate the normalized position clicked
        var clickPosition = (e.pageX - this.offsetLeft) / this.offsetWidth;
        var clickTime = clickPosition * myAudio5.duration;

        // move the playhead to the correct position
        myAudio5.currentTime = clickTime;
        myBufferedTimeRanges = myAudio5.buffered;
        mySeekableTimeRanges = myAudio5.seekable;
      });

      // PLAY 6

      var myAudio6 = document.getElementById('my-audio6');
      var suona6 = document.getElementById('suona6');
      var pause6 = document.getElementById('pause6');
      var bar6 = document.getElementById('bar6');
      var progress6 = document.getElementById('progress6');


      function displayControls6() {
        suona6.style.display = "block";
      }

      // check that the media is ready before displaying the controls
      if (myAudio6.paused) {
        displayControls6();
      } else {
        // not ready yet - wait for canplay event
        myAudio6.addEventListener('canplay', function() {
          displayControls6();
        });
      }

      suona6.addEventListener('click', function() {
        myAudio6.play();
        suona6.style.display = "none";
        pause6.style.display = "block";

        pause.style.display = "none";
        suona.style.display = "block";
        pause2.style.display = "none";
        suona2.style.display = "block";
        pause3.style.display = "none";
        suona3.style.display = "block";
        pause4.style.display = "none";
        suona4.style.display = "block";
        pause5.style.display = "none";
        suona5.style.display = "block";
      });

      pause6.addEventListener('click', function() {
        myAudio6.pause();
        pause6.style.display = "none";
        suona6.style.display = "block";
      });

      // display progress

      myAudio6.addEventListener('timeupdate', function() {
        //sets the percentage
        bar6.style.width = parseInt(((myAudio6.currentTime / myAudio6.duration) * 100), 10) + "%";
      });

      progress6.addEventListener('click', function(e) {

        // calculate the normalized position clicked
        var clickPosition = (e.pageX - this.offsetLeft) / this.offsetWidth;
        var clickTime = clickPosition * myAudio6.duration;

        // move the playhead to the correct position
        myAudio6.currentTime = clickTime;
        myBufferedTimeRanges = myAudio6.buffered;
        mySeekableTimeRanges = myAudio6.seekable;
      });

      //CLOSE

    }
\$\endgroup\$
3
  • \$\begingroup\$ "too many lines" is an opinion that the particular function/code block may be harder to read and/or understand due to its length. This "opinion" is a configuration value set in the "linter" (code analyzer) used in JSFiddle. It does not say "I detected a bug but I'm going to run it anyway. Good luck!" \$\endgroup\$
    – radarbob
    Feb 21 at 0:50
  • 1
    \$\begingroup\$ Sometimes the play / pause mechanism gets stuck => Code Review posts require working code. See "Help" link in the lower right of this page. \$\endgroup\$
    – radarbob
    Feb 21 at 3:05
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Please check that I haven't misrepresented your code, and correct it if I have. \$\endgroup\$ Feb 21 at 8:12

1 Answer 1

1
\$\begingroup\$

And also JS Fiddle reports "this function has too many statements (78)". Would there be a way to reduce the statements?

  • See my comment about "too many lines"

  • The code with: // check that the media is ready before displaying the controls

    • Looks smelly to me. Adding eventListeners in a condition? Doesn't make sense. And it's executed immediately because it is not in a method/function.
    • event handlers are setting each others' properties. I'd bet this, along with the bullet above, is a big reason the app "gets stuck."
  • WARNING! Given that confusion, don't assume I've reorganized style setting code completely correctly. A AudioPlayers collection event handler may very well set all of that.

Instantiate AudioPlayer objects. This will reduce total code ~ 80%. This structure will facilitate cleaning away that "loose code" and coordinate the control display and hide settings

function AudioPlayer (audio, suona, pause, bar, progress) {

  this.myAudio  = document.getElementById(audio);
  this.suona    = document.getElementById(suona);
  this.pause    = document.getElementById(pause);
  this.bar      = document.getElementById(bar);
  this.progress = document.getElementById(progress);
  
  // for unique reference in a collection of AudioPlayers
  this.id = audio;
  
  this.addListeners();
  this.displayControls();
  
} // Player

Turn the methods into instance methods. This code block is not inside the AudioPlayer object.

AudioPlayer.prototype.displayControls() {
   // code here
}

AudioPlayer.prototype.hideControls() {
   // do your thing
}

// register all object event handlers in this method, just
// for convenience.
// DO NOT SET OTHER CONTROL'S VALUES. Do that in the collection object

AudioPlayer.prototype.addListeners() {
  this.myAudio.addEventListener('timeupdate', function(event) {
    //sets the percentage
    event.target.bar.style.width = parseInt(((this.myAudio.currentTime / myAudio3.duration) * 100), 10) + "%";
  });
      
  this.suona.addEventListener('click', function(event) {
    event.target.myAudio.play();
  });
  
  this.progress.addEventListener('click', function(e) {
  // calculate the normalized position clicked
     var clickPosition = (e.pageX - this.offsetLeft) / this.offsetWidth;
     var clickTime = clickPosition * this.myAudio.duration;
 
  // move the playhead to the correct position
     this.myAudio.currentTime = clickTime;
     myBufferedTimeRanges = this.myAudio.buffered;
     mySeekableTimeRanges = this.myAudio.seekable;
  });

  myAudio.addEventListener('canplay', function() {
     displayControls();
  });


} // addEventListeners

Create a collection for easy control setting coordination.

  • I suspect an event handler in this AutoPlayers collection can, in a simple loop, hide or unhide all the controls based on the event.target

  • Remove all the code setting other controls' properties from all event handlers above. You'll set them in AudioPlayers event handler.

.

// AudioPlayer collection for coordinating event behaviour
function AudioPlayers () {
  this.players = []
  
  this.add = function (newPlayer) {
    if(newPlayer) {
      players[newPlayer.id] = newPlayer;
    }
  }

  // event handler pseudo-code

   this.clickHandler = function (event) {
       this.players.each(player => 
           if (player.id == event.target.id) {
              player.displayControls();
           } else {
              player.hideControls();
           }
    } // clickHandler

} // AudioPlayers

Instantiate all the objects

var audioPlayers = new AudioPlayers();
audioPlayers.Add (new AudioPlayer('my-audio', 'suona', 'pause', 'bar', 'progress' ));
audioPlayers.Add (new AudioPlayer('my-audio2', 'suona2', 'pause2', 'bar2', 'progress2' ));
audioPlayers.Add (new AudioPlayer('my-audio3', 'suona3', 'pause3', 'bar3', 'progress3' ));
audioPlayers.Add (new AudioPlayer('my-audio4', 'suona4', 'pause4', 'bar4', 'progress4' ));
audioPlayers.Add (new AudioPlayer('my-audio5', 'suona5', 'pause5', 'bar5', 'progress5' ));
audioPlayers.Add (new AudioPlayer('my-audio6', 'suona6', 'pause6', 'bar6', 'progress6' ));
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.