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\$
3
  • \$\begingroup\$ stackoverflow.com/questions/1470488/… \$\endgroup\$ Commented 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\$ Commented Aug 16, 2012 at 13:10
  • \$\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\$ Commented 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\$ Commented 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\$ Commented Sep 7, 2012 at 17:52

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.