3
\$\begingroup\$

Here is a small code snippet I wrote that I feel is pretty naive. I copied the code directly from my project and renamed some ID/class names.

I'd like to know how you could help to improve/refactor the code. radiobox1 and radiobox2 form a pair, and radiobox3 and radiobox4 form another pair - they're both mutually exclusive - which means select radiobox1 then radiobox2 is disabled, and vice versa. Then I will load the data in terms of the user's selection combination. In total, there are 4 selection combinations.

var viz = (function () {
    var config = {
        width: 960,
        height:600,
        big: true,
        small: true,
        index: 0
    };

    var emptyGraph = function () {
        if (!$('#graph').is(':empty')) {
            $("#graph").empty();
        }
    };

    var bind = function () {
        $("#radiobox1").change(function () {
           if ($(this).is(':checked')) {
               emptyGraph();
               $("#radiobox2").prop("checked", false);
               config.small = true;
               loader.loadData(config);
           }
        });

        $("#radiobox2").change(function () {
            if ($(this).is(':checked')) {
                emptyGraph();
                $("#radiobox1").prop("checked", false);
                config.small = false;
                loader.loadData(config);

            }
        });

        $("#radiobox3").change(function () {
            if ($(this).is(':checked')) {
                emptyGraph();
                $("#radiobox4").prop("checked", false);
                config.big = true;
                loader.loadData(config);
            }
        });


        $("#radiobox4").change(function () {
            if ($(this).is(':checked')) {
                emptyGraph();
                $("#radiobox3").prop("checked", false);
                config.big = false;
                config.index = 1;
                loader.loadData(config);
            }
        });

    };

    return {
        init: bind,
    }

})();
\$\endgroup\$
  • 3
    \$\begingroup\$ Would you mind including the corresponding HTML as well, so that we can better understand what it's doing? (You can use Ctrl-M in the editor to make a live demo.) \$\endgroup\$ – 200_success Aug 19 '15 at 5:33
  • 4
    \$\begingroup\$ Radio buttons can already be mutually exclusive - just give the input's the same name (in your case, one name for a pair and another for the other) and the browser will make sure that only one is selected. Or am I misunderstanding something? \$\endgroup\$ – ᔕᖺᘎᕊ Aug 19 '15 at 11:18
3
\$\begingroup\$

Perhaps a radio button isn't even the input you want to use: your code suggests that these two states are either true or false, which semantically fits the function of a checkbox. Therefore your bind function could look like this:

var bind = function () {
    $("#checkbox1").change(function () {
       emptyGraph();
       config.small = $(this).is(':checked');
       loader.loadData(config);
    });

    $("#checkbox2").change(function () {
        emptyGraph();
        config.big = $(this).is(':checked');
        loader.loadData(config);
    });
};

Going one step further, because the two controls are behaving so similarly, you could combine them:

var bind = function () {
    $('#checkbox1, #checkbox2').change(function () {
       emptyGraph();
       config.small = $('#checkbox1').is(':checked');
       config.big = $('#checkbox2').is(':checked');
       loader.loadData(config);
    });
};

If for whatever reason (e.g., UX concerns) you cannot use checkboxes instead of radio buttons, @Jonathan's answer solves mutual exclusivity from the HTML side of things. Your binding code could then be as simple as follows:

var bind = function () {
    $('#radiobox1, #radiobox2, #radiobox3, #radiobox4').change(function () {
       emptyGraph();
       config.small = $('#radiobox1').is(':checked');
       config.big = $('#radiobox3').is(':checked');
       loader.loadData(config);
    });
};

I just noticed that radiobox4's change function changes the config's index property. In this case, you'll need to do additional logic inside the function:

var bind = function () {
    $('#radiobox1, #radiobox2, #radiobox3, #radiobox4').change(function () {
        emptyGraph();
        config.small = $('#radiobox1').is(':checked');
        if($('#radiobox3').is(':checked')) {
            config.big = true;
        } else {
            config.big = false;
            config.index = 1;
        }
        loader.loadData(config);
    });
};
\$\endgroup\$
  • \$\begingroup\$ I like your comment, especially suggest me to combine those two methods together. However I think if we combine them, we miss one thing - mutually exclusive. For example, as long as "checkbox2" is checked, "checkbox1" should be unchecked. Any thoughts on this? Thanks! \$\endgroup\$ – catlovespurple Aug 19 '15 at 15:01
  • \$\begingroup\$ checkbox1 is functionally the same as radiobox1 and radiobox2, and checkbox2 is functionally the same as radiobox3 and radiobox4. The mutual exclusion is fulfilled by the fact that a checkbox cannot be both checked and unchecked. As per your scenario, checkbox1 and checkbox2 should not be mutually exclusive from each other. \$\endgroup\$ – Gallant Aug 19 '15 at 15:16
  • \$\begingroup\$ @catlovespurple See my updated answer if you want to keep radio buttons. \$\endgroup\$ – Gallant Aug 19 '15 at 15:34
  • \$\begingroup\$ Thanks!! This is exactly what I want! And after reading your previous post(the checkbox one) I come up with the same solution as you just posted! so the answer is granted to you! \$\endgroup\$ – catlovespurple Aug 19 '15 at 15:49
3
\$\begingroup\$

Radio buttons already group together using the name attribute. I suggest throwing away the code.

<input type="radio" name="group1" value="male">Male<br>
<input type="radio" name="group1" value="female">Female<br><br>

<input type="radio" name="group2" value="1">Active<br>
<input type="radio" name="group2" value="0">Inactive

\$\endgroup\$
  • \$\begingroup\$ yes! this is what I want, mutually exclusive radio button groups in html. However, one thing here is that, I have to refactor the JS code in order to load the right data in terms of user selection. Any comments/improvements on my JS code? \$\endgroup\$ – catlovespurple Aug 19 '15 at 15:03

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.