1
\$\begingroup\$

I have three checboxes in my form. I have to check which one is checked and start the action. What do you think? Is this the easiest way of doing it?

  • Checkbox 1 = cat
  • Checkbox 2 = dog
  • Checkbox 3 = dog

  • Only cat -> form action

  • Cat & any of dogs -> play music
  • Dog & dog -> play music
  • Nothing -> form action

HTML:

<section class="pets">
    <form action="sample_action" method="get">
            <input type="hidden" name="sample_name" value="sample_value">
          <ul>
            <li><input id="checkbox1" type="checkbox" name="pet" value="cat"><label for="checkbox1">Cat</label></li>
            <li><input id="checkbox2" type="checkbox" name="pet" value="dog"><label for="checkbox2">Dog 1</label></li>
            <li><input id="checkbox3" type="checkbox" name="pet" value="dog"><label for="checkbox3">Dog 2</label></li>
          </ul>
          <button type="submit">Check my pets</button>
   </form>
</section>

Solution:

function checkPets () {
  var cat = $('#checkbox1').is(":checked");
  var dog1 = $('#checkbox2').is(":checked");
  var dog2 = $('#checkbox3').is(":checked");
  return !cat &&(dog1 || dog2);
}

$(window).load(function(){
$('.pets form').submit( function(e){
    if(checkPets()) {
        e.preventDefault();
        $('body').append('<audio></audio>');
        $('audio').attr({'src': 'audio.mp3', 'autoplay': 'autoplay'});
    }
});
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! Good job on your first question. \$\endgroup\$
    – SirPython
    Commented Jan 14, 2016 at 23:52

4 Answers 4

1
\$\begingroup\$

Is this the easiest way of doing it?

In my view, yes: I don't see how to dramatically simplify your code.
It only can be (very) slightly reduced using return false; instead of e.preventDefault;.

You also might use the more common $(document).ready() in place of $(window).load().
BTW it'd be more readable to rename your checkPets() function to something like mustPlayMusic(), which reflects that it returns a boolean.

So it results in:

$(document).ready(function(){
  $('.pets form').submit( function(e){
    if(mustPlayMusic()) {
      $('body').append('<audio></audio>');
      $('audio').attr({'src': 'audio.mp3', 'autoplay': 'autoplay'});
      return false;
    }
});

On the other hand, something puzzles me: the checkPets()||mustPlayMusic() function doesn't match the conditions you detailed above.

It should be:

function checkPets () {
  var cat = $('#checkbox1').is(":checked");
  var dog1 = $('#checkbox2').is(":checked");
  var dog2 = $('#checkbox3').is(":checked");
  return (cat && (dog1 || dog2)) || (dog1 && dog2);
}
\$\endgroup\$
1
\$\begingroup\$

I'm worried about

   $('body').append('<audio></audio>');
   $('audio').attr({'src': 'audio.mp3', 'autoplay': 'autoplay'});

This adds a new audio element every time the form is submitted, and all of these get their properties set, because $('audio') returns all audio elements in the page. You are just lucky that autoplay just plays the sound once.

It would be much better to insert the audio elemen outside the submit handler (possibly just add it to the HTML) and use the play() method:

$(window).load(function(){
  var audio = $("<audio>").attr({'src': 'audio.mp3'}).appendTo("body")[0];
  $('.pets form').submit( function(e){
    if(checkPets()) {
        e.preventDefault();
        audio.play();
    }
  });
});
\$\endgroup\$
1
\$\begingroup\$

You can wrap all your code in a self-executing anonymous function, which will stop you from having any problems with variables leaking out of scope.

I would also recommend splitting out the various conditions into their own if statements. This will allow for quicker interpretation of the code and easier adjustments if the requirements change in the future.

(function ($) {

    $('.pets form').submit(
        function (e) {

            e.preventDefault();

            var cat = $('#checkbox1').is(":checked");
            var dog1 = $('#checkbox2').is(":checked");
            var dog2 = $('#checkbox3').is(":checked");

            if (cat && !dog1 && !dog2) {
                $(this).trigger('submit');
            }

            if (cat && (dog1 || dog2)) {
                playMusic();
            }

            if (!cat && dog1 && dog2) {
                playMusic();
            }

            if (!cat && !dog1 && !dog2) {
                $(this).trigger('submit');
            }

        }
    );

    function playMusic() {
        $('body').append('<audio></audio>');
        $('audio').attr({ 'src': 'audio.mp3', 'autoplay': 'autoplay' });
    }

})(jQuery);
\$\endgroup\$
1
\$\begingroup\$

Try this ;)

@TODO update audio file path;

$(function() {
  
  /* cache jQuery object for better performance; */
  var $dog1 = $('#checkbox2');
  var $dog2 = $('#checkbox3');
  var $status = $('#status');

  $('.pets form').submit(function(e){
    $status.text('');
    
    /* check is any of the dog checkbox checcked; */
    if($dog1.is(":checked") || $dog2.is(":checked")){
      $status.text("Play audio; and don't submit form;");
      
      /* don't submit form just append audio and done; */
      e.preventDefault();
      
      /* add audio to body and autoplay it; */
      $('body').append('<audio src="audio.mp3" autoplay="autoplay"></audio>');
    }else{
      /* submit form none of the dog checkbox selected; */
      $status.text('Submitting form; none of the dog selected;');      
    }        
  });
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<section class="pets">
  <form action="sample_action" method="get">
    <input type="hidden" name="sample_name" value="sample_value">
    <ul>
      <li>
        <input id="checkbox1" type="checkbox" name="pet" value="cat">
        <label for="checkbox1">Cat</label>
      </li>
      <li>
        <input id="checkbox2" type="checkbox" name="pet" value="dog">
        <label for="checkbox2">Dog 1</label>
      </li>
      <li>
        <input id="checkbox3" type="checkbox" name="pet" value="dog">
        <label for="checkbox3">Dog 2</label>
      </li>
    </ul>
    <button type="submit">Check my pets</button>
    <div id="status"></div>
  </form>
</section>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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