# Efficiency and length of functions when perfomring similar tasks to classes and IDs

I'm not proficient at jQuery, however can tinker and get basic functionality working as I would like. The below code is performing a number of functions to change states of some elements in my site, and it is all working correctly.

However, I'm sure it is not the most efficient way of achieving this (fairly simple) functionality, so I'm hoping somebody can help me to make this much leaner and more efficient?

I have commented the code to explain what each line should be achieving, essentially adding and removing classes to a a number of elements. As you can see, a lot of the lines are repeated to accommodate the different states outlined in the comments, but I'm sure something perhaps like .siblings could be used to reduce the amount of code.

Any help greatly appreciated!

JS

$(document).ready(function () {$('#menu-button').click(function(){
if ($(this).hasClass('active')) {$(this).removeClass();
// When in initial state, add active class
} else {
$(this).addClass('active'); } // OVERLAY STATES // If in menu state, clicking menu button removes all #overlay classes to return to initial state if ($('#overlay').hasClass('active menu')) {
$('#overlay').removeClass(); // If in services state, clicking menu button removes all #overlay classes and add classes to return to menu state } else if ($('#overlay').hasClass('active services')) {
$('#overlay').removeClass().addClass('active menu'); // If in about state, clicking menu button removes all #overlay classes and add classes to return to menu state } else if ($('#overlay').hasClass('active about')) {
$('#overlay').removeClass().addClass('active menu'); // If in contact state, clicking menu button removes all #overlay classes and add classes to return to menu state } else if ($('#overlay').hasClass('active contact')) {
$('#overlay').removeClass().addClass('active menu'); // When in initial state, add classes to change to menu state } else {$('#overlay').addClass('active menu');
}
return false;
});

$('.services a').click(function(){ // When clicking this menu item, remove all #overlay classes and add classes to return to services state$('#overlay').removeClass().addClass('active services');
// Also, return menu button to initial state
$('#menu-button').removeClass(); return false; });$('.about a').click(function(){
// Same as above but for about state
$('#overlay').removeClass().addClass('active about');$('#menu-button').removeClass();
return false;
});

$('.contact a').click(function(){ // Same as above but for contact state$('#overlay').removeClass().addClass('active contact');
$('#menu-button').removeClass(); return false; });$('.logo').click(function(){
// When clicking .logo, return everythign to initial state
$('#overlay').removeClass();$('#menu-button').removeClass();
return false;
});

});


1. Use toggleClass instead of:
if ($(this).hasClass('active')) {$(this).removeClass();
$(this).addClass('active'); }  2. You can map the event handlers for multiple classes: ['about', 'contact', ...].map(cls => {$(.\${cls} a).click(...)}

• Notice that he's not just removing the active class, he's removing all classes. – Barmar Jun 9 '17 at 22:24
• not sure why you care to check the overlay classes, since in every case you are removing all overlay classes and adding the active menu class, unless you want to hide it. toggleclass should suffice here. – Mike Sep 7 '17 at 18:08