0
\$\begingroup\$
function check_itv(key) {

if (key.length){

    if(key=="left"){
        left_itv = setInterval(left,100);
        check_left= false;
        check_right=true;
        check_up=true;
        check_down=true;
        clearInterval(right_itv);
        clearInterval(down_itv);
        clearInterval(up_itv);
    }

    if(key=="right"){
        right_itv = setInterval(right,100);
        check_left= true;
        check_right=false;
        check_up=true;
        check_down=true;
        clearInterval(left_itv);
        clearInterval(down_itv);
        clearInterval(up_itv);
    }

    if(key=="up"){
        up_itv = setInterval(up,100);
        check_left= true;
        check_right=true;
        check_up=false;
        check_down=true;
        clearInterval(left_itv);
        clearInterval(right_itv);
        clearInterval(down_itv);
    }

    if(key=="down"){
        down_itv = setInterval(down,100);
        check_left= true;
        check_right=true;
        check_up=true;
        check_down=false;
        clearInterval(left_itv);
        clearInterval(right_itv);
        clearInterval(up_itv);
    }

}

}

check_left = true;
check_right = true;
check_up = true;
check_down = true;
left_itv = "";
right_itv = "";
up_itv = "";
down_itv = "";

Try to do shorter and better, Any idea ?

Thanks

\$\endgroup\$
4
  • \$\begingroup\$ stackoverflow.com/questions/1470488/… \$\endgroup\$
    – rlemon
    Aug 16, 2012 at 13:07
  • 2
    \$\begingroup\$ Don't have all of these intervals... have one, and set a variable for the direction. \$\endgroup\$
    – rlemon
    Aug 16, 2012 at 13:10
  • \$\begingroup\$ i know about global & local , thanks @rlemon \$\endgroup\$
    – l2aelba
    Aug 16, 2012 at 13:28
  • \$\begingroup\$ @l2aelba What do you mean with "better"? (Faster? Easier to maintain? More compliant with standards? Working in <old browser>?) What are you trying to accomplish? Why are you not using var: var check_left = true;? Why do you have variables and methods in multiples of 4 and not a single variable for the direction? Etc. \$\endgroup\$
    – ANeves
    Aug 16, 2012 at 14:33

1 Answer 1

1
\$\begingroup\$

Without changing too much, here are some tips on improving your code.

Assumptions:

  • You're working in modern web browser.
  • left, up, down, right are defined else where.

1)

Define all the variables at the top. I'm not sure where the fucntions left, up, down, right are.

2)

Use a switch instead of multiple if else.

3)

Perform an operation to collection if multiple elements share the same functionality. Refer to resetValues()

4)

Try to make all the if conditions as small as possible. Refer to !key.length

Here's what I came up with.

Array.prototype.forEach = Array.prototype.forEach || function(fn, scope) {
    for(var i = 0, len = this.length; i < len; ++i) {
        fn.call(scope || this, this[i], i, this);
    }
};

var self = this,
    check_left, check_right, check_up, check_down,
    left_itv, right_itv, up_itv, down_itv;
    
var resetValues = function(){
    ["left_itv", "right_itv", "up_itv", "down_itv"].forEach(function(el){
        clearInterval( self[el] );
    });
    ["check_left", "check_right", "check_up", "check_down"].forEach(function(el){
        self[el] = true;
    });
};

resetValues();

function check_itv(key) {
    if (!key.length) {
        return;
    }
    resetValues();
    switch( key ){
        case "left":
            left_itv = setInterval(left, 100);
            check_left = false;
            break;
        case "right":
            right_itv = setInterval(right, 100);
            check_right = false;
            break;
        case "up":
            up_itv = setInterval(up, 100);
            check_up = false;
            break;
        case "down":
            down_itv = setInterval(down, 100);
            check_down = false;
    }
}

Alternatively, you could swap the global variables inside a singleton(namespace) that has a reset functions. But that add a bit of complexity.

\$\endgroup\$
2
  • \$\begingroup\$ Nice, but use have to use : var check_left = true; var check_right = true; var check_up = true; var check_down = true; so thats work ! :D \$\endgroup\$
    – l2aelba
    Sep 7, 2012 at 8:27
  • 1
    \$\begingroup\$ @l2aelba Updated the code, so that resetValues() set the check direction values when the script loads. Thanks for the update. \$\endgroup\$ Sep 7, 2012 at 17:52

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