I want to pass this function a True or a False and have the elements listed show (true) or hide (false) on this input.

I am currently using this function...

        function SetElementVisibility(visible) {
        if (visible) {
            $("#Div1").show("slow");
            $("#Div2").show("slow");
            $("#Div3").show("slow");
        }
        else {
            $("#Div1").hide("slow");
            $("#Div2").hide("slow");
            $("#Div3").hide("slow");
        }
    }

But i would prefer to not repeat myself by naming the Div's for each outcome.

How can i refactor this into a more DRY (Don't Repeat Yourself) example?

Thanks, Kohan

I want to pass this function a True or a False and have the elements listed show (true) or hide (false) on this input.

I am currently using this function...

        function SetElementVisibility(visible) {
        if (visible) {
            $("#Div1").show("slow");
            $("#Div2").show("slow");
            $("#Div3").show("slow");
        }
        else {
            $("#Div1").hide("slow");
            $("#Div2").hide("slow");
            $("#Div3").hide("slow");
        }
    }

But i would prefer to not repeat myself by naming the Div's for each outcome.

How can i refactor this into a more DRY (Don't Repeat Yourself) example?

Thanks, Kohan

• javascript
• jquery
• refactoring
• dry

• 1 Javascript doesn't listen to DRY. – Josh K Commented Jul 7, 2010 at 14:00

8 Answers 8

Use square-bracket notation to pick a method name depending on the visible variable:

$('#Div1, #Div2, #Div3')[visible? 'show' : 'hide']('slow');

This should work:

function SetElementVisibility(visible) {
   $("#Div1,#Div2,#Div3")[visible]("slow");
}

// Display all
SetElementVisibility( "show" );

// Hide all
SetElementVisibility( "hide" );

If you dont want to use "show" and "hide" as arguments but true and false you would have to change it a little bit:

function SetElementVisibility(visible) {
   $("#Div1,#Div2,#Div3")[visible?'show':'hide']("slow");
}

// Display all
SetElementVisibility( true );

// Hide all
SetElementVisibility( false );

Do you mean like $("#Div1, #Div2, #Div3").show(...

See this for more information...

use this method:

$("ID").toggle()

Try using class instead of id. For example:

function SetElementVisibility(visible) {
    if (visible) {
        $(".showHideDiv").show("slow");
    }
    else {
        $(".showHideDiv").hide("slow");
    }
}

you can improve this

if(..)
$('[id^=Div]').show();
else
$('[id^=Div]').hide();

if you keep this div start with same id prefix !

Several ways:

$('#Div1, #Div2, #Div3').hide('slow');

or

$('#Div1').add('#Div2').add('#Div3').hide('slow');

or

$.each(['#Div1', '#Div2', '#Div3'], function(i,v){
    $(v).hide('slow');
});

Well ok, the third looks a little bit alien but whatever.

If all those desired divs really start with Div you may also use

$('div[id=^Div]').hide('slow');

Another example:

function SetElementVisibility(visible) {
    $elements =  $("#Div1, #Div2, #Div3");
    if (visible) {
        $elements.show("slow");
    }
    else {
        $elements.hide("slow");
    }
}

If I were you, I will replace with

 function SetElementVisibility() {
       $('#Div1,#Div2,#Div3').toggle();
}