# Keyboard handler for moving in four directions

I'm having trouble deciding if I should leave the default: break; in the switch statement or not. It does kinda show intent, i.e. that most keys are not handled, but it's also effectively useless code. I feel like it's clear enough anyway, but maybe being explicit is better?

function handleKeyDownEvent(event) {
switch (event.keyCode) {
case 87: // w
player.dy = -50
break
case 65: // a
player.dx = -50
break
case 83: // s
player.dy = 50
break
case 68: // d
player.dx = 50
break
default:
break
}
}


Maps seem to be the preferred way of doing this, instead of a switch statement. How does the movement map work with the corresponding handleKeyUpEvent which works the same way, except that it zeroes the dx or dy values?

Should I keep the default: break; or not?

• Usually is a good practice to put a break sentence on every case. I was going to develop a bit more on why but I found a post that resumes it quite well. – A. Romeu Mar 8 '16 at 22:33

Your code is rather short, but can still be greatly improved.

Should I keep the default: break; or not?

Do not keep it. Since you aren't doing anything for the default case, there is no point in having it. You can remove that entire thing.

## Map

In order to improve the readability of your code (and overall efficiency), I recommend creating a map that maps the key code values to the changes needed to be made to dx and dy. Here is what I mean:

var keyToChangeMap = {
87: [0, -50], // [change in x, change in y]
65: [-50, 0]
...
};


Now, you have an $O(1)$ method for finding out how to change the dx and dy values. This is also flexible: you can easily change the values that x and y change by, and you can easily add more keys to listen to.

Then, in your function, simply use the key code to get a value from the map:

function handleKeyDownEvent(event) {
var keyCode = event.keyCode;
if(keyToChangeMap.hasOwnProperty(keyCode)) {
var change = keyToChangeMap[keyCode];
player.dx += change[0];
player.dy += change[1];
}
}


## Magic numbers

You have some magic numbers in your code. It's pretty difficult to understand these numbers, so you should create constants to represent these values.

For example,

var W_KEY = 87;
var A_KEY = 65;


You don't even have to be that specific. You can do something like this:

var UP_KEY = 87;
var LEFT_KEY = 65;


In this case, I think that default: would be noise. It's obvious that this code makes no attempt to handle every possible keycode.

I suggest using a variant of @SirPython's suggestion to use a lookup table. It can be written in a way that reminds you of the keyboard layout.

var MOVEMENT = {    /* W */ 87: { dy: -50 },
/* A */ 65: { dx: -50 },              /* D */ 68: { dx: +50 },
/* S */ 83: { dy: +50 },
};

function handleKeyDownEvent(event) {
var movement;
if (movement = MOVEMENT[event.keyCode]) {
player.dx += (movement.dx || 0);
player.dy += (movement.dy || 0);
}
}