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\$
  • \$\begingroup\$ stackoverflow.com/questions/1470488/… \$\endgroup\$ – rlemon Aug 16 '12 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 '12 at 13:10
  • \$\begingroup\$ i know about global & local , thanks @rlemon \$\endgroup\$ – l2aelba Aug 16 '12 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 '12 at 14:33
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\$
  • \$\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 '12 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\$ – Larry Battle Sep 7 '12 at 17:52

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