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

Is this the right way to work with callbacks and the EventEmitter?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisthewithandwayeventemitterworkcallbacksright

Problem

I'm creating a small online multiplayer game in NodeJS and I'm wondering if I'm "doing it right".
Here is a bit of my code:

// app.js
// if I give an id as argument we assume it's an existing player.
var Player = new player();

Player.on('playerLoaded', function (err, result) {

    if (result) {

        Player.setName('somename');

        Player.on('setName', function (err, result) {

            console.log('Name changed');

        });

    } else {

        // couldn't load the player

    }

});


This is my player object:

function Player(id) {

    // if we don't have a id, this player needs to be created
    this.id = (typeof id !== 'number') ? this.createPlayer() : this.setPlayer(id);
    this.name = "";
    EventEmitter.call(this);

}


This is the createPlayer method:

Player.prototype.createPlayer = function () {

    var self = this;

    // we only need a datetime to insert
    connection.query("INSERT INTO players SET `created_at` = now()", function (err, result) {

        if (result) {

            process.nextTick(function () {

                if (result.insertId) {

                    console.log("Successfully found player no. " + result.insertId);

                    self.id = result.insertId;

                    // everything was successful
                    self.emit('playerLoaded', false, true);

                } else {

                    console.log("Trouble inserting a new player");

                    self.emit('playerLoaded', true, false);

                }

            });

        } else {

            process.nextTick(function () {

                console.log("Trouble with the mysql while inserting player: " + err);

                self.emit('playerLoaded', err, null);

            });

        }

    });

};


And this is the setName method:

```
Player.prototype.setName = function (name) {

var self = this;

connection.query("UPDATE players SET ? WHERE ?", [
{ name: name },

Solution

Interesting question,

I am assuming you know that usually, in essence, the setting of the name would be written like this:

Player.setName('somename' , function( err, result ) {
  console.log('Name changed');    
});


and in essence setName would be

Player.prototype.setName = function( name, callbackFunction ) {

    var self = this;

    connection.query("UPDATE players SET ? WHERE ?", [
        { name: name },
        { id: self.id }
    ], function (err, result) {

        if (result) {
            process.nextTick(function () {
                self.name = name;
                console.log("Successfully updated name of player no. " + self.id);
                callbackFunction( false, true );
            });
        } else {
            process.nextTicket(function () {
                console.log("Trouble with the mysql while changing player name: " + err);
                callbackFunction( err, false );
            });
        }
    });
};


I like that with your approach you avoid "callback hell". Since your project is a small game I would advise you to see how far you can go with this scheme.

Other than that, I hope you are not planning to write a whole function including SQL code for each property. In my mind you should only have 1 function that takes care of updating the whole of the player object.

Finally, easy up on the new lines, it makes your code too hard to read.

Code Snippets

Player.setName('somename' , function( err, result ) {
  console.log('Name changed');    
});
Player.prototype.setName = function( name, callbackFunction ) {

    var self = this;

    connection.query("UPDATE players SET ? WHERE ?", [
        { name: name },
        { id: self.id }
    ], function (err, result) {

        if (result) {
            process.nextTick(function () {
                self.name = name;
                console.log("Successfully updated name of player no. " + self.id);
                callbackFunction( false, true );
            });
        } else {
            process.nextTicket(function () {
                console.log("Trouble with the mysql while changing player name: " + err);
                callbackFunction( err, false );
            });
        }
    });
};

Context

StackExchange Code Review Q#40267, answer score: 3

Revisions (0)

No revisions yet.