1
\$\begingroup\$

I have a jQuery function using a series of if and else statements. I am wondering if there is a more elegant way I can code this. I am thinking I could create an array with my "hashtag names" and then loop it through a function. However, I am not quite sure how to go about doing that.

setTimeout(function() {
    var i = 0;
    if(window.location.hash == "#" + "somename"){
        var i = 0;
        return scroll(i);
    }

    else if(window.location.hash == "#" + "somethingelse"){
        var i = 1;
        return scroll(i);
    }

    else if(window.location.hash == "#" + "someotherword"){
        var i = 2;
        return scroll(i);
    }

    else if (window.location.hash == "#" + "sometext"){
        var i = 3;
        return scroll(i);
    }
}, 500 );

function scroll(i){
    var title = $('.title').eq(i);
    var top =  $(title).offset().top - 50 ;
    $('html,body').animate({ scrollTop: top }, 'slow');
}

I'm checking to see what hashtag is in the URL, and then scrolling to the part of the page that is related to that hashtag. This method works, but I would like to optimize the if/else part of my code if possible. An array with a loop may not be the best approach. I am open to any ideas.

\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

You can just parse the number right out of the location like this:

setTimeout(function() {
    var matches = window.location.hash.match(/^#page(\d+)$/);
    if (matches) {
        scroll(+matches[1] - 1);
    }
}, 500);

OK, now that you've changed to random strings, you could use a table lookup like this:

setTimeout(function() {
    var values = {
        "#somename": 1,
        "#somethingelse": 2,
        "#someotherword": 3,
        "#sometext": 4
    };
    var val = values[window.location.hash];
    if (val) {
        scroll(val - 1);
    }
}, 500 );

P.S. Note, there is no reason to return anything from the setTimeout() callback as the return value is not processed by anything which is why I removed the return statement.

\$\endgroup\$
7
  • \$\begingroup\$ Sorry, I am actually not using numbers... I just updated my question to reflect that. I put that them there as an example I didn't intend for it be like that, my mistake. For my scenario I am actually using different names for each page. \$\endgroup\$ Commented Aug 13, 2012 at 19:37
  • \$\begingroup\$ @KrisHollenbeck - OK, I've now provided a lookup table option for random strings. \$\endgroup\$
    – jfriend00
    Commented Aug 13, 2012 at 19:46
  • \$\begingroup\$ Thanks, I will see if I can get it to work with my code and get back to you. \$\endgroup\$ Commented Aug 13, 2012 at 19:54
  • \$\begingroup\$ Hmm.. I am thinking I need to post some more code. That doesn't seem to work with what I have... I will update my question \$\endgroup\$ Commented Aug 13, 2012 at 20:04
  • \$\begingroup\$ FYI.. I get this error when trying to implement the "table lookup" .. Uncaught TypeError: Cannot read property 'top' of null \$\endgroup\$ Commented Aug 13, 2012 at 20:08
0
\$\begingroup\$

The book High Performance JavaScript provides some good advice on looping in JavaScript. To quote it:

Generally speaking, if-else is best used when there are two discrete values or a few different ranges of values for which to test. When there are more than two discrete values for which to test, the switch statement is the most optimal choice. ... When there are a large number of discrete values for which to test, both if-else and switch are significantly slower than using a lookup table. (Ch. 4 Algorithms and Flow Control)

Also, instead of calling window.location.hash with each evaluation, copy that value to a local variable and evaluate that. It will improve performance since you won't keep going back to the DOM to look-up the value.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.