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\$
2
  • 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\$ Commented Aug 19, 2015 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\$ Commented Aug 19, 2015 at 11:18

2 Answers 2

Reset to default
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\$
4
  • \$\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\$ Commented Aug 19, 2015 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\$ Commented Aug 19, 2015 at 15:16
  • \$\begingroup\$ @catlovespurple See my updated answer if you want to keep radio buttons. \$\endgroup\$ Commented Aug 19, 2015 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\$ Commented Aug 19, 2015 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\$
1
  • \$\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\$ Commented Aug 19, 2015 at 15:03

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.