Setting currency and language values

Question

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');
function reloadPage() {
 if(hasChanged) {
 alert('reload now');
 } 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();
});

Answer 1

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.

• Brian: 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.
• konijn: I actually wrote exactly that and then removed it.
• user1732521: Thank you, I've applied your suggestions to my code (I've edited my original post)
• Dave Jarvis: @tomdemuyt: "results indicate a significant improvement in time and lower visual effort with the underscore style" ~ cs.kent.edu/~jmaletic/papers/…
• konijn: @DaveJarvis Sure, but for JavaScript the standard is camelCase, when in Rome..

Answer 2

.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();

• user1732521: 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 :)