You are not consistently applying the 2nd good technique, for example this:
function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { old_direction = direction; } break; case 'right': if(old_direction!='left') { old_direction = direction; } break; case 'up': if(old_direction!='down') { old_direction = direction; } break; case 'down': if(old_direction!='up') { old_direction = direction; } break; } }could simply have been
function setWay(direction) { var oppositeDirection = { left : 'right', right: 'left', up: 'down', down:'up' } if( direction != oppositeDirection[old_direction] ){ old_direction = direction; } }I will leave deep thoughts to on whether
- You want to specify that
'left'is the opposite of'right', since you already specified'right'is the opposite of'left' - Whether you would want to merge
oppositeDirectionandkeys
- You want to specify that
You copy pasted some code in 'giveLife
andgiveLifegrow` andgrowthat could also benefit from the above approach. I would have written this:switch(old_direction) { case 'right': nextPosition[0] += 1; break; case 'left': nextPosition[0] -= 1; break; case 'up': nextPosition[1] -= 1; break; case 'down': nextPosition[1] += 1; break; }as
//2 properly named array indexes for x and y var X = 0; var Y = 1; //vectors for each direction var vectors = { right : { x : 1 , y : 0 }, left : { x : -1 , y : 0 }, up : { x : 0 , y : -1 }, down : { x : 0 , y : 1 } } function updatePosition( direction ){ var vector = vectors( direction ); if( vector ){ nextPosition[X] += vector.x; nextPosition[Y] += vector.y; } else{ throw "Invalid direction: " + direction } }The advantages here are:
- If you wanted to play snake with 8 directions, you could
- No silent failure if an invalid direction is passed along
The following code gives me the creeps:
function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); } else if(element.mozRequestFullScreen) { element.mozRequestFullScreen(); } else if(element.webkitRequestFullscreen) { element.webkitRequestFullscreen(); } else if(element.msRequestFullscreen) { element.msRequestFullscreen(); } }have you considered using something like
function launchFullscreen(e) { var request = e.requestFullscreen || e.mozRequestFullScreen || e.webkitRequestFullscreen || e.msRequestFullscreen; request(); }This is also is not a pretty sight:
document.addEventListener("fullscreenchange", function(){snake.game.adjust();}); document.addEventListener("webkitfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("mozfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("MSFullscreenChange", function(){snake.game.adjust();});That should at least be
document.addEventListener("fullscreenchange", snake.game.adjust ); document.addEventListener("webkitfullscreenchange", snake.game.adjust ); document.addEventListener("mozfullscreenchange", snake.game.adjust ); document.addEventListener("MSFullscreenChange", snake.game.adjust );and really there has to be a better way than to subscribe to every browser event ;) I am assuming you are not simply providing
snake.game.adjustbecause it is not yet initialized at that point. I would rather solve that problem then creating functions to deal with that problem.
You are not consistently applying the 2nd good technique, for example this:
function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { old_direction = direction; } break; case 'right': if(old_direction!='left') { old_direction = direction; } break; case 'up': if(old_direction!='down') { old_direction = direction; } break; case 'down': if(old_direction!='up') { old_direction = direction; } break; } }could simply have been
function setWay(direction) { var oppositeDirection = { left : 'right', right: 'left', up: 'down', down:'up' } if( direction != oppositeDirection[old_direction] ){ old_direction = direction; } }I will leave deep thoughts to on whether
- You want to specify that
'left'is the opposite of'right', since you already specified'right'is the opposite of'left' - Whether you would want to merge
oppositeDirectionandkeys
- You want to specify that
You copy pasted some code in 'giveLife
andgrow` that could also benefit from the above approach. I would have written this:switch(old_direction) { case 'right': nextPosition[0] += 1; break; case 'left': nextPosition[0] -= 1; break; case 'up': nextPosition[1] -= 1; break; case 'down': nextPosition[1] += 1; break; }as
//2 properly named array indexes for x and y var X = 0; var Y = 1; //vectors for each direction var vectors = { right : { x : 1 , y : 0 }, left : { x : -1 , y : 0 }, up : { x : 0 , y : -1 }, down : { x : 0 , y : 1 } } function updatePosition( direction ){ var vector = vectors( direction ); if( vector ){ nextPosition[X] += vector.x; nextPosition[Y] += vector.y; } else{ throw "Invalid direction: " + direction } }The advantages here are:
- If you wanted to play snake with 8 directions, you could
- No silent failure if an invalid direction is passed along
The following code gives me the creeps:
function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); } else if(element.mozRequestFullScreen) { element.mozRequestFullScreen(); } else if(element.webkitRequestFullscreen) { element.webkitRequestFullscreen(); } else if(element.msRequestFullscreen) { element.msRequestFullscreen(); } }have you considered using something like
function launchFullscreen(e) { var request = e.requestFullscreen || e.mozRequestFullScreen || e.webkitRequestFullscreen || e.msRequestFullscreen; request(); }This is also is not a pretty sight:
document.addEventListener("fullscreenchange", function(){snake.game.adjust();}); document.addEventListener("webkitfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("mozfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("MSFullscreenChange", function(){snake.game.adjust();});That should at least be
document.addEventListener("fullscreenchange", snake.game.adjust ); document.addEventListener("webkitfullscreenchange", snake.game.adjust ); document.addEventListener("mozfullscreenchange", snake.game.adjust ); document.addEventListener("MSFullscreenChange", snake.game.adjust );and really there has to be a better way than to subscribe to every browser event ;) I am assuming you are not simply providing
snake.game.adjustbecause it is not yet initialized at that point. I would rather solve that problem then creating functions to deal with that problem.
You are not consistently applying the 2nd good technique, for example this:
function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { old_direction = direction; } break; case 'right': if(old_direction!='left') { old_direction = direction; } break; case 'up': if(old_direction!='down') { old_direction = direction; } break; case 'down': if(old_direction!='up') { old_direction = direction; } break; } }could simply have been
function setWay(direction) { var oppositeDirection = { left : 'right', right: 'left', up: 'down', down:'up' } if( direction != oppositeDirection[old_direction] ){ old_direction = direction; } }I will leave deep thoughts to on whether
- You want to specify that
'left'is the opposite of'right', since you already specified'right'is the opposite of'left' - Whether you would want to merge
oppositeDirectionandkeys
- You want to specify that
You copy pasted some code in
giveLifeandgrowthat could also benefit from the above approach. I would have written this:switch(old_direction) { case 'right': nextPosition[0] += 1; break; case 'left': nextPosition[0] -= 1; break; case 'up': nextPosition[1] -= 1; break; case 'down': nextPosition[1] += 1; break; }as
//2 properly named array indexes for x and y var X = 0; var Y = 1; //vectors for each direction var vectors = { right : { x : 1 , y : 0 }, left : { x : -1 , y : 0 }, up : { x : 0 , y : -1 }, down : { x : 0 , y : 1 } } function updatePosition( direction ){ var vector = vectors( direction ); if( vector ){ nextPosition[X] += vector.x; nextPosition[Y] += vector.y; } else{ throw "Invalid direction: " + direction } }The advantages here are:
- If you wanted to play snake with 8 directions, you could
- No silent failure if an invalid direction is passed along
The following code gives me the creeps:
function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); } else if(element.mozRequestFullScreen) { element.mozRequestFullScreen(); } else if(element.webkitRequestFullscreen) { element.webkitRequestFullscreen(); } else if(element.msRequestFullscreen) { element.msRequestFullscreen(); } }have you considered using something like
function launchFullscreen(e) { var request = e.requestFullscreen || e.mozRequestFullScreen || e.webkitRequestFullscreen || e.msRequestFullscreen; request(); }This is also is not a pretty sight:
document.addEventListener("fullscreenchange", function(){snake.game.adjust();}); document.addEventListener("webkitfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("mozfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("MSFullscreenChange", function(){snake.game.adjust();});That should at least be
document.addEventListener("fullscreenchange", snake.game.adjust ); document.addEventListener("webkitfullscreenchange", snake.game.adjust ); document.addEventListener("mozfullscreenchange", snake.game.adjust ); document.addEventListener("MSFullscreenChange", snake.game.adjust );and really there has to be a better way than to subscribe to every browser event ;) I am assuming you are not simply providing
snake.game.adjustbecause it is not yet initialized at that point. I would rather solve that problem then creating functions to deal with that problem.
You are not consistently applying the 2nd good technique, for example this:
function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { old_direction = direction; } break; case 'right': if(old_direction!='left') { old_direction = direction; } break; case 'up': if(old_direction!='down') { old_direction = direction; } break; case 'down': if(old_direction!='up') { old_direction = direction; } break; } }could simply have been
function setWay(direction) { var oppositeDirection = { left : 'right', right: 'left', up: 'down', down:'up' } if( direction != oppositeDirection[old_direction] ){ old_direction = direction; } }I will leave deep thoughts to on whether
- You want to specify that
'left`` is the opposite of 'left''right' is the opposite of, since you already specified 'right''right``, since you already specified'right'is the opposite of'left' - Whether you would want to merge
oppositeDirectionandkeys
- You want to specify that
You copy pasted some code in 'giveLife
andgrow` that could also benefit from the above approach. I would have written this:switch(old_direction) { case 'right': nextPosition[0] += 1; break; case 'left': nextPosition[0] -= 1; break; case 'up': nextPosition[1] -= 1; break; case 'down': nextPosition[1] += 1; break; }as
//2 properly named array indexes for x and y var X = 0; var Y = 1; //vectors for each direction var vectors = { right : { x : 1 , y : 0 }, left : { x : -1 , y : 0 }, up : { x : 0 , y : -1 }, down : { x : 0 , y : 1 } } function updatePosition( direction ){ var vector = vectors( direction ); if( vector ){ nextPosition[X] += vector.x; nextPosition[Y] += vector.y; } else{ throw "Invalid direction: " + direction } }The advantages here are:
- If you wanted to play snake with 8 directions, you could
- No silent failure if an invalid direction is passed along
The following code gives me the creeps:
function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); } else if(element.mozRequestFullScreen) { element.mozRequestFullScreen(); } else if(element.webkitRequestFullscreen) { element.webkitRequestFullscreen(); } else if(element.msRequestFullscreen) { element.msRequestFullscreen(); } }have you considered using something like
function launchFullscreen(e) { var request = e.requestFullscreen || e.mozRequestFullScreen || e.webkitRequestFullscreen || e.msRequestFullscreen; request(); }This is also is not a pretty sight:
document.addEventListener("fullscreenchange", function(){snake.game.adjust();}); document.addEventListener("webkitfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("mozfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("MSFullscreenChange", function(){snake.game.adjust();});That should at least be
document.addEventListener("fullscreenchange", snake.game.adjust ); document.addEventListener("webkitfullscreenchange", snake.game.adjust ); document.addEventListener("mozfullscreenchange", snake.game.adjust ); document.addEventListener("MSFullscreenChange", snake.game.adjust );and really there has to be a better way than to subscribe to every browser event ;) I am assuming you are not simply providing
snake.game.adjustbecause it is not yet initialized at that point. I would rather solve that problem then creating functions to deal with that problem.
You are not consistently applying the 2nd good technique, for example this:
function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { old_direction = direction; } break; case 'right': if(old_direction!='left') { old_direction = direction; } break; case 'up': if(old_direction!='down') { old_direction = direction; } break; case 'down': if(old_direction!='up') { old_direction = direction; } break; } }could simply have been
function setWay(direction) { var oppositeDirection = { left : 'right', right: 'left', up: 'down', down:'up' } if( direction != oppositeDirection[old_direction] ){ old_direction = direction; } }I will leave deep thoughts to on whether
- You want to specify that
'left`` is the opposite of'right', since you already specified'right`` is the opposite of'left' - Whether you would want to merge
oppositeDirectionandkeys
- You want to specify that
You copy pasted some code in 'giveLife
andgrow` that could also benefit from the above approach. I would have written this:switch(old_direction) { case 'right': nextPosition[0] += 1; break; case 'left': nextPosition[0] -= 1; break; case 'up': nextPosition[1] -= 1; break; case 'down': nextPosition[1] += 1; break; }as
//2 properly named array indexes for x and y var X = 0; var Y = 1; //vectors for each direction var vectors = { right : { x : 1 , y : 0 }, left : { x : -1 , y : 0 }, up : { x : 0 , y : -1 }, down : { x : 0 , y : 1 } } function updatePosition( direction ){ var vector = vectors( direction ); if( vector ){ nextPosition[X] += vector.x; nextPosition[Y] += vector.y; } else{ throw "Invalid direction: " + direction } }The advantages here are:
- If you wanted to play snake with 8 directions, you could
- No silent failure if an invalid direction is passed along
The following code gives me the creeps:
function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); } else if(element.mozRequestFullScreen) { element.mozRequestFullScreen(); } else if(element.webkitRequestFullscreen) { element.webkitRequestFullscreen(); } else if(element.msRequestFullscreen) { element.msRequestFullscreen(); } }have you considered using something like
function launchFullscreen(e) { var request = e.requestFullscreen || e.mozRequestFullScreen || e.webkitRequestFullscreen || e.msRequestFullscreen; request(); }This is also is not a pretty sight:
document.addEventListener("fullscreenchange", function(){snake.game.adjust();}); document.addEventListener("webkitfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("mozfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("MSFullscreenChange", function(){snake.game.adjust();});That should at least be
document.addEventListener("fullscreenchange", snake.game.adjust ); document.addEventListener("webkitfullscreenchange", snake.game.adjust ); document.addEventListener("mozfullscreenchange", snake.game.adjust ); document.addEventListener("MSFullscreenChange", snake.game.adjust );and really there has to be a better way than to subscribe to every browser event ;) I am assuming you are not simply providing
snake.game.adjustbecause it is not yet initialized at that point. I would rather solve that problem then creating functions to deal with that problem.
You are not consistently applying the 2nd good technique, for example this:
function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { old_direction = direction; } break; case 'right': if(old_direction!='left') { old_direction = direction; } break; case 'up': if(old_direction!='down') { old_direction = direction; } break; case 'down': if(old_direction!='up') { old_direction = direction; } break; } }could simply have been
function setWay(direction) { var oppositeDirection = { left : 'right', right: 'left', up: 'down', down:'up' } if( direction != oppositeDirection[old_direction] ){ old_direction = direction; } }I will leave deep thoughts to on whether
- You want to specify that
'left'is the opposite of'right', since you already specified'right'is the opposite of'left' - Whether you would want to merge
oppositeDirectionandkeys
- You want to specify that
You copy pasted some code in 'giveLife
andgrow` that could also benefit from the above approach. I would have written this:switch(old_direction) { case 'right': nextPosition[0] += 1; break; case 'left': nextPosition[0] -= 1; break; case 'up': nextPosition[1] -= 1; break; case 'down': nextPosition[1] += 1; break; }as
//2 properly named array indexes for x and y var X = 0; var Y = 1; //vectors for each direction var vectors = { right : { x : 1 , y : 0 }, left : { x : -1 , y : 0 }, up : { x : 0 , y : -1 }, down : { x : 0 , y : 1 } } function updatePosition( direction ){ var vector = vectors( direction ); if( vector ){ nextPosition[X] += vector.x; nextPosition[Y] += vector.y; } else{ throw "Invalid direction: " + direction } }The advantages here are:
- If you wanted to play snake with 8 directions, you could
- No silent failure if an invalid direction is passed along
The following code gives me the creeps:
function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); } else if(element.mozRequestFullScreen) { element.mozRequestFullScreen(); } else if(element.webkitRequestFullscreen) { element.webkitRequestFullscreen(); } else if(element.msRequestFullscreen) { element.msRequestFullscreen(); } }have you considered using something like
function launchFullscreen(e) { var request = e.requestFullscreen || e.mozRequestFullScreen || e.webkitRequestFullscreen || e.msRequestFullscreen; request(); }This is also is not a pretty sight:
document.addEventListener("fullscreenchange", function(){snake.game.adjust();}); document.addEventListener("webkitfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("mozfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("MSFullscreenChange", function(){snake.game.adjust();});That should at least be
document.addEventListener("fullscreenchange", snake.game.adjust ); document.addEventListener("webkitfullscreenchange", snake.game.adjust ); document.addEventListener("mozfullscreenchange", snake.game.adjust ); document.addEventListener("MSFullscreenChange", snake.game.adjust );and really there has to be a better way than to subscribe to every browser event ;) I am assuming you are not simply providing
snake.game.adjustbecause it is not yet initialized at that point. I would rather solve that problem then creating functions to deal with that problem.
From a once over:
Good
- I like how you use an IIFE
- I really like how you use
direction = keys[event.keyCode];
Not so good
You are not consistently applying the 2nd good technique, for example this:
function setWay(direction) { switch(direction) { case 'left': if(old_direction!='right') { old_direction = direction; } break; case 'right': if(old_direction!='left') { old_direction = direction; } break; case 'up': if(old_direction!='down') { old_direction = direction; } break; case 'down': if(old_direction!='up') { old_direction = direction; } break; } }could simply have been
function setWay(direction) { var oppositeDirection = { left : 'right', right: 'left', up: 'down', down:'up' } if( direction != oppositeDirection[old_direction] ){ old_direction = direction; } }I will leave deep thoughts to on whether
- You want to specify that
'left`` is the opposite of'right', since you already specified'right`` is the opposite of'left' - Whether you would want to merge
oppositeDirectionandkeys
- You want to specify that
You copy pasted some code in 'giveLife
andgrow` that could also benefit from the above approach. I would have written this:switch(old_direction) { case 'right': nextPosition[0] += 1; break; case 'left': nextPosition[0] -= 1; break; case 'up': nextPosition[1] -= 1; break; case 'down': nextPosition[1] += 1; break; }as
//2 properly named array indexes for x and y var X = 0; var Y = 1; //vectors for each direction var vectors = { right : { x : 1 , y : 0 }, left : { x : -1 , y : 0 }, up : { x : 0 , y : -1 }, down : { x : 0 , y : 1 } } function updatePosition( direction ){ var vector = vectors( direction ); if( vector ){ nextPosition[X] += vector.x; nextPosition[Y] += vector.y; } else{ throw "Invalid direction: " + direction } }The advantages here are:
- If you wanted to play snake with 8 directions, you could
- No silent failure if an invalid direction is passed along
The following code gives me the creeps:
function launchFullscreen(element) { if(element.requestFullscreen) { element.requestFullscreen(); } else if(element.mozRequestFullScreen) { element.mozRequestFullScreen(); } else if(element.webkitRequestFullscreen) { element.webkitRequestFullscreen(); } else if(element.msRequestFullscreen) { element.msRequestFullscreen(); } }have you considered using something like
function launchFullscreen(e) { var request = e.requestFullscreen || e.mozRequestFullScreen || e.webkitRequestFullscreen || e.msRequestFullscreen; request(); }This is also is not a pretty sight:
document.addEventListener("fullscreenchange", function(){snake.game.adjust();}); document.addEventListener("webkitfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("mozfullscreenchange", function(){snake.game.adjust();}); document.addEventListener("MSFullscreenChange", function(){snake.game.adjust();});That should at least be
document.addEventListener("fullscreenchange", snake.game.adjust ); document.addEventListener("webkitfullscreenchange", snake.game.adjust ); document.addEventListener("mozfullscreenchange", snake.game.adjust ); document.addEventListener("MSFullscreenChange", snake.game.adjust );and really there has to be a better way than to subscribe to every browser event ;) I am assuming you are not simply providing
snake.game.adjustbecause it is not yet initialized at that point. I would rather solve that problem then creating functions to deal with that problem.