patternjavascriptModerate
Two keyboard handlers for a video game character
Viewed 0 times
handlerscharactervideotwogameforkeyboard
Problem
The code below is equivalent. I can see pros and cons for both versions. Which is better: the short, clever way, or the long, ctrl+c way?
Short version:
Long version:
Short version:
character.on("key",function(key){
var action = ({
"a":{axis:"x",direction:-1},
"d":{axis:"x",direction:1},
"w":{axis:"y",direction:1},
"s":{axis:"y",direction:-1}})[key[1]],
stop = key[0]=="-";
if (action)
if (stop)
this.walkdir[action.axis] = 0;
else
this.walkdir[action.axis] = this.lookdir[action.axis] = action.direction;
});Long version:
character.on("key",function(key){
switch (key){
case "+a":
this.walkdir.x = -1;
this.lookdir.x = -1;
break;
case "+d":
this.walkdir.x = 1;
this.lookdir.x = 1;
break;
case "+w":
this.walkdir.y = 1;
this.lookdir.y = 1;
break;
case "+s":
this.walkdir.y = -1;
this.lookdir.y = -1;
break;
case "-a":
if (this.walkdir.x == -1)
this.walkdir.x = 0;
break;
case "-d":
if (this.walkdir.x == 1)
this.walkdir.x = 0;
break;
case "-w":
if (this.walkdir.y == 1)
this.walkdir.y = 0;
break;
case "-s":
if (this.walkdir.y == -1)
this.walkdir.y = 0;
break;
case "space":
this.setStance("jumping");
break;
};
});Solution
The short code wins hands down. I understood it immediately, and more importantly, I can trivially verify that the code is reasonably error free. This is much harder with the longer code.
You say that the longer code is easier to understand but I claim that this is objectively wrong.
Case in point, the long code uses lots of magic numbers: 1, 0, -1, … what do these stand for? Ah, the short code tells us: they are
The longer code also makes us scroll (depending on the screen size) to see the whole method. This significantly impacts ease of understanding. I believe there were even studies demonstrating this empirically (but I cannot cite them; Code Complete would probably be the relevant reference here).
The one thing I would change in the short code is the lookup itself: define the dictionary separately, maybe even outside the method, and perform the lookup as follows:
And maybe think about tokenising
You also said that the longer code is more efficient but I’d like to see a benchmark before I believe that. You probably think that the first code is slower because of the dictionary lookup. But consider that JavaScript is a dynamic language – every single variable access is potentially a dictionary lookup internally. So there is no difference in performance – indeed, the short code could be faster since there’s less variable lookup involved.
(Of course the two code snippets do different things: the long version handles jumping, and they behave differently when the character was previously walking in one direction and now you cancel walking into a different direction.)
You say that the longer code is easier to understand but I claim that this is objectively wrong.
Case in point, the long code uses lots of magic numbers: 1, 0, -1, … what do these stand for? Ah, the short code tells us: they are
directions.The longer code also makes us scroll (depending on the screen size) to see the whole method. This significantly impacts ease of understanding. I believe there were even studies demonstrating this empirically (but I cannot cite them; Code Complete would probably be the relevant reference here).
The one thing I would change in the short code is the lookup itself: define the dictionary separately, maybe even outside the method, and perform the lookup as follows:
var action = movement_commands[key[1]];And maybe think about tokenising
key properly, i.e. assigning the parts to variables before using them. However, I think that the method is short enough to make this unnecessary.You also said that the longer code is more efficient but I’d like to see a benchmark before I believe that. You probably think that the first code is slower because of the dictionary lookup. But consider that JavaScript is a dynamic language – every single variable access is potentially a dictionary lookup internally. So there is no difference in performance – indeed, the short code could be faster since there’s less variable lookup involved.
(Of course the two code snippets do different things: the long version handles jumping, and they behave differently when the character was previously walking in one direction and now you cancel walking into a different direction.)
Code Snippets
var action = movement_commands[key[1]];Context
StackExchange Code Review Q#25308, answer score: 13
Revisions (0)
No revisions yet.