Dynamically changing available select options

Question 1

I have the following code working here on JSFIDDLE:

<!DOCTYPE html>
<html>
<head>
 <meta http-equiv="X-UA-Compatible" content="IE=edge">
 <script src="//ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
</head>
<body>
<label for="opt1"><input class="radio-button-selector" type="radio" name="account" id="opt1" value="opt1">Opt1</label>
<label for="opt2"><input class="radio-button-selector" type="radio" name="account" id="opt2" value="opt2">Opt2</label>
<div id="form_select" class="form-group">
 <select id="term" name="term" class="form-control">
 <option value="0">Please select</option>
 <option value="6">6 months</option>
 <option value="9">9 months</option>
 <option value="10">10 months</option>
 <option value="12">12 months</option>
 <option value="18">18 months</option>
 <option value="24">24 months</option>
 <option value="30">30 months</option>
 <option value="36">36 months</option>
 <option value="42">42 months</option>
 <option value="48">48 months</option>
 </select>
</div>
<script type="text/javascript">
if (window.jQuery) {
// jQuery is available.
}
else { console.log("-- no jquery --"); }
</script>
<script type="text/javascript">
$(document).ready(function() {
 $("input.radio-button-selector").change(function(){ 
 console.log("Changed.");
 if ( $(this).val() == "opt2" ) {
 $("#term option[value='48']").hide();
 $("#term option[value='42']").hide();
 }
 else if ( $(this).val() == "opt1" ) {
 $("#term option[value='48']").show();
 $("#term option[value='42']").show();
 }
 });
 });
</script>
</body>
</html>

Questions:

Any way to improve the jQuery code? I saw a lot of times what surprisingly elegant things can be done in JS
Any way to improve the html code?
I only use the class "input-button-selector" for the JavaScript, is this actually needed or could I select the radio buttons in a different way?

Note that this code won't work on IE 8/9/10 (jQuery is not loading)

Question 2

@ProgramFOX If he is not asking for us to fix it, but it works on a major browser (Chrome/FireFox/Safari), then that's okay

Question 3

This is a classic example of counting on the UI to keep track of state and model.

Because you know there are only 2 options, and you know how to go from one option to another, you simply hide and add options. Nowhere in your code is explicitly declared what durations are linked to which option.

If you dont care about that, you could optimize the code doing something like this:

$(document).ready(function() {
 var monthsDifference = $("#term option[value='48'], #term option[value='42']");
 $("input.radio-button-selector").change(function(){ 
 if ( $(this).val() == "opt2" ) {
 monthsDifference.hide();
 }
 else if ( $(this).val() == "opt1" ) {
 monthsDifference.show();
 }
 });
 });

I would advise you to care though, and have the code know what the months are and build selectors for each option. Something like this (untested):

$(document).ready(function() {
 var optionMonths = {
 opt1: [6,9,10,12,18,24,30,36,42,48],
 opt2: [6,9,10,12,18,24,30,36]
 }
 //Cache all options
 var $options = $("#term option");
 $("input.radio-button-selector").change(function(){ 
 //Hide all options first
 $options.hide(); 
 //Then show relevant options
 showOptionMonths( optionMonths[ $(this).val()] );
 });
});
function showOptionMonths( months ){
 months.forEach( function( month ){
 $("#term option[value='" + month + "']").show();
 });
}

Of course, I would try to find a better name than opt1 or opt2, but I dont have enough knowledge of what you are trying to achieve to suggest something more meaningful.

Finally, you could cache the selectors for each option, I leave that as an exercise for the reader.

Question 4

1) What is the improvement? Can you explain why is better? 2) Why not test it before posting, as I did post the fiddle, you could just fork it? 3) opt1 and opt2 names are used just for privacy, production code has relevant names.

Question 5

4) "This is a classic example of counting on the UI to keep track of state and model." Is this a bad or a good thing?

Question 6

5) "Nowhere in your code is explicitly declared what durations are linked to which option." Not sure what you mean and why this is bad.

Question 7

6) "you could cache the selectors for each option" I think var monthsDifference = $("#term option[value='48'], #term option[value='42']"); and var $options = $("#term option"); in your code are examples of caching the selectors. Am I wrong?

Question 8

For the next maintainer it is bad to not have the links between the options and the months. From a software maintenance perspective, the second option is better.But, if you are comfortable by only caching 48 and 42, then you can go for that.

konijn konijn 34.2k5 gold badges70 silver badges267 bronze badges · Accepted Answer · 2014-11-21 17:38:41Z

This is a classic example of counting on the UI to keep track of state and model.

Because you know there are only 2 options, and you know how to go from one option to another, you simply hide and add options. Nowhere in your code is explicitly declared what durations are linked to which option.

If you dont care about that, you could optimize the code doing something like this:

$(document).ready(function() {
 var monthsDifference = $("#term option[value='48'], #term option[value='42']");
 $("input.radio-button-selector").change(function(){ 
 if ( $(this).val() == "opt2" ) {
 monthsDifference.hide();
 }
 else if ( $(this).val() == "opt1" ) {
 monthsDifference.show();
 }
 });
 });

I would advise you to care though, and have the code know what the months are and build selectors for each option. Something like this (untested):

$(document).ready(function() {
 var optionMonths = {
 opt1: [6,9,10,12,18,24,30,36,42,48],
 opt2: [6,9,10,12,18,24,30,36]
 }
 //Cache all options
 var $options = $("#term option");
 $("input.radio-button-selector").change(function(){ 
 //Hide all options first
 $options.hide(); 
 //Then show relevant options
 showOptionMonths( optionMonths[ $(this).val()] );
 });
});
function showOptionMonths( months ){
 months.forEach( function( month ){
 $("#term option[value='" + month + "']").show();
 });
}

Of course, I would try to find a better name than opt1 or opt2, but I dont have enough knowledge of what you are trying to achieve to suggest something more meaningful.

Finally, you could cache the selectors for each option, I leave that as an exercise for the reader.

1) What is the improvement? Can you explain why is better? 2) Why not test it before posting, as I did post the fiddle, you could just fork it? 3) opt1 and opt2 names are used just for privacy, production code has relevant names.
4) "This is a classic example of counting on the UI to keep track of state and model." Is this a bad or a good thing?
5) "Nowhere in your code is explicitly declared what durations are linked to which option." Not sure what you mean and why this is bad.
6) "you could cache the selectors for each option" I think var monthsDifference = $("#term option[value='48'], #term option[value='42']"); and var $options = $("#term option"); in your code are examples of caching the selectors. Am I wrong?
For the next maintainer it is bad to not have the links between the options and the months. From a software maintenance perspective, the second option is better.But, if you are comfortable by only caching 48 and 42, then you can go for that.

Stack Exchange Network

Dynamically changing available select options

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

Dynamically changing available select options

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions