Keypress function conditional

Question

How can I combine these if statements to make this a little cleaner and be less repetitive?

$(document).keydown(function(e){
    if (e.keyCode == 38) {
        if($html.hasClass('client-loading') || canAnim === false) {
            return false;
        }
        var $prevProj = $('.project-current').prev('.project');
        $prevProj.click();
    }

    if (e.keyCode == 40) {
        if($html.hasClass('client-loading') || canAnim === false) {
            return false;
        }
        var $nextProj = $('.project-current').next('.project');
        $nextProj.click();
    }

    // Prevent rapid clicking
    if ( e.keyCode == 38 || e.keycode == 40 ) {
        canAnim = false;

        setTimeout(function(){
            canAnim = true;
        },2000);
    }
});

Answer 1

Here's another one, just for the heck of it

$(document).keydown(function (e) {
  var current;

  // return early
  if( e.keyCode !== 38 && e.keyCode !== 40 ) {
    return false;
  }

  // again, return early
  if( $html.hasClass('client-loading') || canAnim === false ) {
    return false;
  }

  current = $('.project-current');

  if( e.keyCode === 38 ) {
    current.prev('.project').click();
  }

  if( e.keyCode === 40 ) {
    current.next('.project').click();
  }

  canAnim = false;
  setTimeout(function () { canAnim = true; }, 2000);
});

Answer 2

Less readable, but another way.

var prevNext = (e.keyCode == 38) ? "prev" : 
                   (e.keyCode == 40) : "next" : null;
if ( prevNext && !$html.hasClass('client-loading') && canAnim !== false ) {
    $('.project-current')[prevNext]('.project').click();   
    canAnim = false;
    setTimeout(function() { canAnim = true;},2000); }
}

Answer 3

This is about as concise as i think it will reasonably get. There might be a better way to handle double presses then a global variable timeout.

if ( e.keyCode == 38 || e.keycode == 40 ) 
{
    if( $html.hasClass('client-loading') || canAnim === false ) return false;

    if (e.keyCode == 38) var $prevProj = $('.project-current').prev('.project').click();
    if (e.keyCode == 40) var $nextProj = $('.project-current').next('.project').click(); 
    canAnim = false;
    setTimeout(function() { canAnim = true;},2000); }

}

Answer 4

Another variation. I've taken the liberty of altering the method of timing, but that can easily be reverted.

$(document).keydown(function(e) {
    var now = (new Date()).getTime(), direction = ({38: 'prev', 40: 'next'})[e.keyCode];
    if (!direction || $html.hasClass('client-loading') || now < lastPressed + 2000) {
        return false;
    }
    lastPressed = now;
    $('.project-current')[direction]('.project').click();
}​

Answer 5

$(document).keydown(function (e) {

    //there is a very minor overhead when accessing properties
    //or querying elements in jQuery
    //cache the value in a variable if they are to be used repeatedly
    //
    //if they don't change during the course of the page's life,
    //or by dynamic means, cache them outside the handler
    //
    //I'll just assume .project-current is dynamic and will need requerying
    //everytime we call the handler

    var keycode = e.keyCode,
        loading = $html.hasClass('client-loading'),
        currentProject = $('.project-current');

    //run the code only on 38 and 40, when not loading and can animate
    //otherwise do nothing
    //why would you run code when it shouldn't?

    //when testing for boolean, it makes no sense comparing to true and false
    //you can use the variable directly to represent it's state

    if((keycode===38 || keycode===40) && !loading && canAnim ){

        //there is a minor optimization done when using "else"
        //when the first condition is met, conditions chained with else
        //will not be evaluated anymore. This means process time savings
        //without it, the second condition will still be evaluated

        if(keycode===38){
            currentProject.prev('.project').click();
        } else if(keyCode===40){
            currentProject.next('.project').click();
        }

        canAnim = false;
        setTimeout(function(){
            canAnim = true;
        },2000);

    }        
});

Answer 6

The switch statement is made exactly for this kind of case.

$(document).keydown(function(e){
    var $proj;
    switch (e.keyCode) {
        case 38:
            if($html.hasClass('client-loading') || canAnim === false) {
                return false;
            }
            $proj = $('.project-current').prev('.project');
            break;

        case 40:
            if($html.hasClass('client-loading') || canAnim === false) {
                return false;
            }
            $proj = $('.project-current').next('.project');
            break;

        // Prevent rapid clicking
        case 38:
        case 40:
            canAnim = false;

            setTimeout(function(){
                canAnim = true;
            },2000);
            break;
    }

    $proj.click();
});

You can also notice the small refactoring I applied. Keeps some common stuff out of the switch, so that less code is in each case.