# Setting currency and language values

I'd like to improve my code, but I'm not sure where to start. Everything works as I want it to; I just think it's ugly.

I'll split this into two parts:

1. I have two drop-downs that I'll explain later. In these drop-downs, I have options to set currency and language. I use a hidden form to set the current values with PHP, and extract the current value from these field. I then compare if the selected value matches the initial one.

2. Within my two drop-downs, clicking on one or the other closes the other one, and open itself if it's not open. Upon closing the drop-down, I check if I should submit the form (reloadPage()).

I think this should be split into separate functions, but I don't really know how to merge all of this together. I'm not asking for a full recode of my stuff, but I'd appreciate how to make this less ugly.

Part 1

$(function() { hasChanged = false; var$options = $('#options'); var$currencycode = $options.find('#currency_code'); var$languagecode = $options.find('#language_code'); var$initCurrencycode = $currencycode.val(); console.log($initCurrencycode);
$('.monnaie').find('a').click(function() {$this = $(this); if(!$this.hasClass('selected')) {
$this.parent().siblings().find('a').removeClass('selected');$myCurrencycode = $this.attr('class');$currencycode.val($myCurrencycode);$this.addClass('selected');
if($myCurrencycode ==$initCurrencycode) {
hasChanged = false;
console.log(hasChanged);
} else {
hasChanged = true;
console.log(hasChanged);
}
}
});
});


Part 2

$optionsdropdown =$('#mobile-options');
$menudropdown =$('#mobile-menu');
if(hasChanged) {
} else {
console.log('nope');
}
}
$('.options-toggler').click( function() { if(!$optionsdropdown.hasClass('hide')) {
$optionsdropdown.addClass('hide'); } else {$optionsdropdown.removeClass('hide');
}
if(!$menudropdown.hasClass('hide')) {$menudropdown.addClass('hide');
}
});
$('.menu-toggler').click( function() { if(!$menudropdown.hasClass('hide')) {
$menudropdown.addClass('hide'); } else {$menudropdown.removeClass('hide');
}
if(!$optionsdropdown.hasClass('hide')) {$optionsdropdown.addClass('hide');
}
});
$('html').click(function() { if(!$optionsdropdown.hasClass('hide')) {
$optionsdropdown.addClass('hide'); } if(!$menudropdown.hasClass('hide')) {
$menudropdown.addClass('hide'); reloadPage(); } });$('#mobile-menu, .menu-toggler').click(function(event){
event.stopPropagation();
});
$('#mobile-options, .options-toggler').click(function(event){ event.stopPropagation(); });  ## 2 Answers My 2 cents: • Use jQuery toggleClass : http://api.jquery.com/toggleClass/ • Use camelCase : currencycode -> currencyCode etc. • Use English everywhere : 'monnaie' should not be there • Booleans are awesome  if($myCurrencycode == $initCurrencycode) { hasChanged = false; console.log(hasChanged); } else { hasChanged = true; console.log(hasChanged); }  should be  hasChanged = ($myCurrencycode != $initCurrencycode) console.log( hasChanged );  • Finally, if(!$menuDropdown.hasClass('hide')) {
$menuDropdown.addClass('hide'); }  can be replaced with $menuDropdown.addClass('hide'); it will not add the class multiple times.

• OP isn't even using hasChanged, so he could also go a step further and use console.log($myCurrencycode !=$initCurrencycode);. Though that's starting to approach the edge between brevity and readability. Aug 29 '13 at 16:43
• I actually wrote exactly that and then removed it. Aug 29 '13 at 18:03
• Thank you, I've applied your suggestions to my code (I've edited my original post) Aug 29 '13 at 18:47
• @tomdemuyt: "results indicate a significant improvement in time and lower visual effort with the underscore style" ~ cs.kent.edu/~jmaletic/papers/… Aug 29 '13 at 19:59
• @DaveJarvis Sure, but for JavaScript the standard is camelCase, when in Rome.. Aug 29 '13 at 20:21

.addClass() won't add a class multiple times, so this:

if(!$optionsDropdown.hasClass('hide')) {$optionsDropdown.addClass('hide');
}


can be rewritten as:

$optionsDropdown.addClass('hide');  But why even use a hide class at all ? jQuery has .hide() and .show() helper functions, so you can just write: $optionsDropdown.hide();

• Because I'm applying the class in the source, I find it more semantic to hide a dropdown menu with a class than with hide(). I might be wrong, but I don't know. I will update my code to use addClass once, thanks :) Aug 30 '13 at 13:58