HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Two keyboard handlers for a video game character

Submitted by: @import:stackexchange-codereview··
0
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:

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 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.