Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 5 of 5
  1. #1
    New to the CF scene
    Join Date
    Jan 2013
    Posts
    2
    Thanks
    1
    Thanked 0 Times in 0 Posts

    Efficient way of using if and else.

    Hi there! I'm pretty new to Javascript and I was wondering if there was a better method to handle the situation. I'm making a website that uses Javascript to hide and show content via DIV boxes.

    It's working great but the only issue is that I think there might be a better way to make it more efficient as the more 'pages' / 'DIV boxes' involved, the longer the code. Here's the current code I have:

    Code:
    function myself() {
    	if($('#myself').css('display') == 'block') {
    		$('#myself').hide();
    		$('#banner').fadeIn();
    	}
    	else if($('#about').css('display') == 'block') {
    		$('#about').hide();
    		$('#myself').fadeIn();
    	}
    	else if($('#animation').css('display') == 'block') {
    		$('#animation').hide();
    		$('#myself').fadeIn();
    	}
    	else {
    		$('#banner').hide();
    		$('#myself').fadeIn();
    	}
    }
    
    function about() {
    	if($('#about').css('display') == 'block') {
    		$('#about').hide();
    		$('#banner').fadeIn();
    	}
    	else if($('#myself').css('display') == 'block') {
    		$('#myself').hide();
    		$('#about').fadeIn();
    	}
    	else if($('#animation').css('display') == 'block') {
    		$('#animation').hide();
    		$('#about').fadeIn();
    	}
    	else {
    		$('#banner').hide();
    		$('#about').fadeIn();
    	}
    	
    }
    
    function animation() {
    	if($('#animation').css('display') == 'block') {
    		$('#animation').hide();
    		$('#banner').fadeIn();
    	}
    	else if($('#about').css('display') == 'block') {
    		$('#about').hide();
    		$('#animation').fadeIn();
    	}
    		else if($('#myself').css('display') == 'block') {
    		$('#myself').hide();
    		$('#animation').fadeIn();
    	}
    	else {
    		$('#banner').hide();
    		$('#animation').fadeIn();
    	}

  • #2
    Senior Coder
    Join Date
    Jan 2011
    Location
    Missouri
    Posts
    4,159
    Thanks
    23
    Thanked 599 Times in 598 Posts
    one or two else if's is OK by me but longer lists get the SWITCH Statement:
    http://www.w3schools.com/js/js_switch.asp

  • #3
    Regular Coder
    Join Date
    Jan 2013
    Location
    Germany
    Posts
    578
    Thanks
    4
    Thanked 77 Times in 77 Posts
    @ sunfighter

    A switch statement isn't that easy to use in this particular case because he's not just iffing over one variable.

    @ doughavant

    First things first: Learn to cache your jQuery variables! Google is your friend. This is a standard improvement known to have a dramatic effect on performance (in particular mobile devices, but the bigger the site, the more important it is for PCs too).

    As for the question itself, I agree that this if/else stuff doesn't look too great (working on a big corporate software everyday I'm getting used to it, though – unfortunately). What you want to do is unify those functions as they all pretty much do the same. This solution is a bit tricky – does it do what you want it to do? I'm asking because it's not 100% equivalent to your code, but if it's supposed to do what I think it is, it should (hopefully) work. You can see it being put to use here: http://jsfiddle.net/K8pTU/

    Code:
    function showPage( page ) {
        var matchedItself = $( '#myself, #about, #animation, #banner' )
            .filter(function () {
                return $( this ).css( 'display' ) === 'block';
            })
            .first()
            .hide()
            .is( $( '#' + page ) );
    
        if( matchedItself ) {
            $( '#banner' ).fadeIn();
        } else {
            $( '#' + page ).fadeIn();
        }
    }
    As you can see, it avoids if/else/switch constructs almost completely and rather utilizes some (still fairly simple) "jQuery magic". Reading it carefully should make it easy to understand, but if you need further explanations on how it works, just ask.
    Last edited by Airblader; 01-30-2013 at 08:40 PM.

  • Users who have thanked Airblader for this post:

    doughavant (01-31-2013)

  • #4
    New to the CF scene
    Join Date
    Jan 2013
    Posts
    2
    Thanks
    1
    Thanked 0 Times in 0 Posts
    Quote Originally Posted by Airblader View Post
    @ sunfighter

    A switch statement isn't that easy to use in this particular case because he's not just iffing over one variable.

    @ doughavant

    First things first: Learn to cache your jQuery variables! Google is your friend. This is a standard improvement known to have a dramatic effect on performance (in particular mobile devices, but the bigger the site, the more important it is for PCs too).

    As for the question itself, I agree that this if/else stuff doesn't look too great (working on a big corporate software everyday I'm getting used to it, though – unfortunately). What you want to do is unify those functions as they all pretty much do the same. This solution is a bit tricky – does it do what you want it to do? I'm asking because it's not 100% equivalent to your code, but if it's supposed to do what I think it is, it should (hopefully) work. You can see it being put to use here: http://jsfiddle.net/K8pTU/

    Code:
    function showPage( page ) {
        var matchedItself = $( '#myself, #about, #animation, #banner' )
            .filter(function () {
                return $( this ).css( 'display' ) === 'block';
            })
            .first()
            .hide()
            .is( $( '#' + page ) );
    
        if( matchedItself ) {
            $( '#banner' ).fadeIn();
        } else {
            $( '#' + page ).fadeIn();
        }
    }
    As you can see, it avoids if/else/switch constructs almost completely and rather utilizes some (still fairly simple) "jQuery magic". Reading it carefully should make it easy to understand, but if you need further explanations on how it works, just ask.
    Hey thanks for the great tip! I had trouble understanding your code as a whole but parts of it actually helped me out! Here's what I came up with:

    Code:
    function show(selector) {
      if ($(selector).is(":visible")) {
        $(selector).hide();
        $("#banner").fadeIn();
      } else {
        $("#banner,.hide").hide();
        $(selector).fadeIn();
      }
    }
    
    function myself() {
    	show("#myself");
    }
    
    function about() {
    	show("#about");
    }
    
    function animation() {
    	show("#animation");
    }
    What do you think? It answered my question but the thing is, is this the most practical method? As I do plan to add new content, I somehow think this is still flawed. I'm still trying to understand your code completely (sorry! total college CS newb here :<)

  • #5
    Regular Coder
    Join Date
    Jan 2013
    Location
    Germany
    Posts
    578
    Thanks
    4
    Thanked 77 Times in 77 Posts
    Your code is still bad because you still add a new function for every page (I just assume they're pages). Although you could get rid of all those functions (animation(), myself(), …) and simply call show(...) directly?

    If your code does, what you want it to do, it's fine. I'd just suggest you use what I just wrote in the paragraph above.


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •