2
\$\begingroup\$

I had posted this on StackOverflow — someone pointed me here.

My code is working, but it's incredibly verbose and I know that it can be made more compact. I just don't know how :-)

I'm building an audio portfolio. When a button is clicked, a sequence of events happens. When another button is clicked, all other active buttons are killed and the sequence for that specific button runs.

The buttons are invisible and are placed on a visualisation of a switch. When clicked, an image of the switch flicked into its "activated" state has its class of "display: none" removed. That should give the user the impression that actually flicking a switch starts playing audio.

Like so:

$(function(){

  // FIRST BUTTON
  $('.button01').click(function() {
    if ($('.switch01').hasClass('activated')){

    // So if button.button01 is clicked and img.switch01 has class "activated"

      // Kill all audio
      $('audio').each(function(){ this.pause(); this.currentTime = 0; });

      // Turn this switch off
      $('.switch01').removeClass('activated');

      // Kill all info cards showing the playback controls
      $('.audio-info-card').addClass('d-none');
    } else { 


    // If button.button01 is clicked and img.switch01 DOESN'T have class "activated"

      // Turn all other switches off
      $('.switch02, .switch03').removeClass('activated');

      // Kill all info cards
      $('.audio-info-card').addClass('d-none');

      // Activate this switch and info card
      $('.switch01').addClass('activated');
      $('.audio-info-card#card01').removeClass('d-none');

      // Kill all audio
      $('audio').each(function(){ this.pause(); this.currentTime = 0; });

      // Start this audio
      $('#audio01-player')[0].play();
    }
  });

  // SECOND BUTTON
  $('.button02').click(function() {
    if ($('.switch02').hasClass('activated')){ 

    // So if button.button02 is clicked and img.switch02 has class "activated"

      // Kill all audio
      $('audio').each(function(){ this.pause(); this.currentTime = 0; });

      // Turn this switch off
      $('.switch02').removeClass('activated');

      // Kill all info card showing the playback controls
      $('.audio-info-card').addClass('d-none');
    } else { 


    // If button.button02 is clicked and img.switch02 DOESN'T have class "activated"

      // Turn all other switches off
      $('.switch01, .switch03').removeClass('activated');

      // Kill all info cards
      $('.audio-info-card').addClass('d-none');

      // Activate this switch and info card
      $('.switch02').addClass('activated');
      $('.audio-info-card#card02').removeClass('d-none');

      // Kill all audio
      $('audio').each(function(){ this.pause(); this.currentTime = 0; });

      // Start this audio
      $('#audio02-player')[0].play();
    }
  });

There are 16 buttons. I realize this code is stupid but JS / jQuery isn't my strong suit :-D

Fortunately, the code works, but any help making this simpler would be greatly appreciated!

\$\endgroup\$
1
\$\begingroup\$

Factor out detailed code into subfunctions

One relatively mechanical thing you could do is transform all of the commented blocks into functions. For a small section, something like this

  // Kill all audio
  $('audio').each(function(){ this.pause(); this.currentTime = 0; });

  // Turn this switch off
  $('.switch01').removeClass('activated');

  // Kill all info cards showing the playback controls
  $('.audio-info-card').addClass('d-none');

becomes

  killAllAudio();
  turnSwitchOff( '.switch01' );
  killAllInfoCards();

  //...

function killAllAudio(){
  $('audio').each(function(){ this.pause(); this.currentTime = 0; });
}

function turnSwitchOff( switch ){
  $(switch).removeClass('activated');
}

// Kill all info cards showing the playback controls
function killAllInfoCards(){
  $('.audio-info-card').addClass('d-none');
}

After this, your top-level algorithm should be much clearer. At the top level it should almost look like pseudocode, except the comments are all calls to subfunctions which do one small named task.

Factoring out code into subfunctions also helps you see "more code" by abstracting away small details. With shorter functions, you can see more of them on the screen at once.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks, I'll give that a try! \$\endgroup\$ – kvkooten May 20 at 21:04
  • \$\begingroup\$ This helped — makes everything more legible indeed. Next step is to combine functions because code still is far from DRY. \$\endgroup\$ – kvkooten May 22 at 14:22

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.