I would code it like this:
$.fn.showIf = function( show, arg ) {
return this[ show ? 'show' : 'hide' ]( arg );
};
$(function() {
var $epc = $('#edit-profile-country');
function setVisibility() {
var value = $epc.val();
$('#state').showIf( value == 'US' );
$('#province').showIf( value == 'CA' );
}
setVisibility();
$epc.change( setVisibility );
});
The one-line .showIf() plugin makes it easy to either show or hide an
element depending on a condition.
The $epc variable is there merely to avoid repeating that selector in the
code. Since the state and province selectors are only used once each, I
didn't create variables for them.
About the second question, checking for the existence of the state/province
dropdowns: With this code (or your original code), there is no need to check
whether these exist. If they don't exist on the page, the show/hide/showIf
functions will silently do nothing. (Those functions, like most jQuery
methods, operate on each of the selected DOM elements - and it's perfectly
OK to select zero elements.)
If you do want to check whether one of those elements exists, you can do it
like this:
if( $('#state').length ) {
/* exists */
}
else {
/* missing */
}
-Mike
> From: John H
>
> I'm not new to web development or javascript, but am
> relatively new to jQuery. I'm wondering if someone would be
> willing to quickly review the following code for efficiency
> and best practices. The function works as required, but I'd
> like to take this as a learning opportunity.
>
> In a nutshell the code initially hides two divs onload and
> then the function shows/hides the state/province divs
> depending on the value of the country dropdown. I want to
> keep the code readable, but I know it could be tighten up.
>
> The state/province divs have a style attribute to initially hide them.
> Exp. style="display: none;"
>
> Questions:
>
> 1) Can the duplicate show/hide methods be reduced?
> 2) What about checking for the existence of the
> state/province dropdowns? (I know they're always going to be
> there because of the context of the page, but want to know
> what the best practice is.)
>
> $(document).ready(function() {
> // set default div visibility
> if ($('#edit-profile-country').val() == 'US') {
> $('#state').show();
> }
> if ($('#edit-profile-country').val() == 'CA') {
> $('#province').show();
> }
> $('#edit-profile-country').change(function() {
> $('#state').hide();
> $('#province').hide();
> if ($(this).val() == 'US') {
> $('#state').show();
> }
> if ($(this).val() == 'CA') {
> $('#province').show();
> }
> })
> });
>
