refactor 3 lines of javascript to minimize code

Question

I am looking to refactor the three lines of code in the else part of the conditional, you'll see where I have it commented.

You'll notice a naming convention for the id's: id, id-div, as seen in the first line: club-community-service, club-community-service-id

I want to shorten that up. I thought maybe storing all names into an array, then looping thru them like (below). If there is a better way to go about this, I am all ears, thanks!

//in theory
array names = [names...];
foreach(names as n) {
 $("#" + n + "-div").toggle($("#" + n).get(0).checked);
}
$('#high-school-student').bind('click', function() {
 if($("input[id=high-school-student]").is(":checked")) {
 $('.field.full.club-community-service, .field.full.other-campus-activities, .field.full.community-public-service, .field.full.other-community-event-activity, .field.honors-program, .field.full.out-of-hs-5-years').hide();
 $('#club-community-service-div, #other-campus-activities-div, #community-public-service-div, #other-community-event-activity-div').hide();
 } else {
 $('.field.full.club-community-service, .field.full.other-campus-activities, .field.full.community-public-service, .field.full.other-community-event-activity, .field.honors-program, .field.full.out-of-hs-5-years').show();
 // ------ RIGHT HERE - best way to refactor these next three lines
 $("#club-community-service-div").toggle($('#club-community-service').get(0).checked);
 $("#other-campus-activities-div").toggle($('#other-campus-activities').get(0).checked);
 $("#community-public-service-div").toggle($('#community-public-service').get(0).checked);
 }
});
$('.watch-for-toggle').bind('click', function() {
 var id = $(this).attr('id');
 $('#' + id + '-div').toggle();
});

• Caridorc: As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. For examples of good titles, check out Best of Code Review 2014 - Best Question Title Category You may also want to read How to get the best value out of Code Review - Asking Questions.

Answer 1

I didn't figure out a way to shorten those three lines without being too tangled (without modifying the html), but I would rewrite your code in this way:

$('#high-school-student').bind('click', function() {
 var highSchoolStudent = $(this).is(":checked");
 $('.field.full.club-community-service, .field.full.other-campus-activities, .field.full.community-public-service, .field.full.other-community-event-activity, .field.honors-program, .field.full.out-of-hs-5-years').toggle(!highSchoolStudent);
 if (highSchoolStudent) { 
 $('#club-community-service-div, #other-campus-activities-div, #community-public-service-div, #other-community-event-activity-div').hide();
 } else {
 //Use a multiselector, convert the result to array and iterate over it.
 //I used replace to remove the '-div'. A regular expression would be a more elegant solution.
 $.each($('#club-community-service-div, #other-campus-activities-div, #community-public-service-div').toArray(), function(i,v) { 
 $(v).toggle($(v.id.replace('-div', '')).is(':checked'))
 });​​​​​​​​​​​​​​​​​​​
 }
});

• ANeves: You can do $('#foo, #bar').each(function(elem() {});, it's better and more concise.
• Jose S: @ANeves is right. You can use: $('#club-community-service-div, #other-campus-activities-div, #community-public-service-div').each(function(i,elem) {$(elem).toggle(elem.id.replace('-div',''))});

Answer 2

If you have control over the div tags and can make the html something like this:

<div id='club-community-service-div' data-rel='club-community-service' class='check-toggle'>

then you can do something like this:

$('.check-toggle').toggle(function () { return document.getElementById($(this).data('rel')).checked; });

However, a much better change to this code would be to cache the various DOM traversals.

Answer 3

@BillBarry proposes a good way to address your problem.

Other improvements

1. Write $("input#high-school-student") instead of $("input[id=high-school-student]");
2. But in your particular use, write $(this), since the element is the one raising the event;
3. Use JQuery 1.7's .on() instead of .bind();
4. What if the checkbox changes without being clicked? You should bind to other events as well: input (mind IE!), change, etc.
5. You can use .toggle(bool) to show/hide the element list, instead of doing the if and repeating the selector;
6. You should cache and re-use the JQuery DOM elements, where possible, instead of repeatedly getting them (but avoid polluting the global scope);
7. Naming an id foo-div raises alarm bells, it's "hungarian notation" for HTML! Why aren't you just targeting #foo > div or #foo div instead?? Even if you have other divs in there, you can give it a class - #foo > .bar. Or if it's adjacent, #foo + div. Etc.;

5:

var isHighSchoolStudent = $(this).is(':checked');
$('.field.full.club-community-service, .field.full.other-campus-activities, .field.full.community-public-service, .field.full.other-community-event-activity, .field.honors-program, .field.full.out-of-hs-5-years'
 ).toggle(!isHighSchoolStudent);