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

Question

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 },
        { id: self.id }
    ], function (err, result) {

        if (result) {

            process.nextTick(function () {

                self.name = name;

                console.log("Successfully updated name of player no. " + self.id);

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

            });

        } else {

            process.nextTicket(function () {

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

                self.emit('setName', err, false);

            });


        }

    });


};

Basically my question is if this is the right way to deal with callbacks?

Answer 1

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.

Answer 2

Very interesting question, here is my opinion.

I don't think EventEmitter as a way to deal or avoid callbacks. There are two main scenarios where I use EventEmitter instead of callbacks:

1. When I need to execute (in your case setName) in one place and subscribe in another place.
2. When the function has more end states or intermediate states that I'm interested on.

So, if you only are interested when the name is changed and you always will call on('setName immediately I consider that EventEmitter doesn't worth for this case and will be better to just pass a function:

Player.setName('newname', function (err) {
  console.log('named changed');
});

Now, a use case where I'd use an emitter can be something like this:

var players = [];

app.post('/player', function (req, res) {

  //handle the creation of the player
  //and how to react to different events
  var player = new Player(req.body);

  player
    .on('setName', function (oldname, newname) {
      players.forEach(function (p) {
        p.notify(oldname + ' changed name to ' + newname);
      });
    }).on('quit', function () {
      //remove the player from the array of players
      players.remove(players.indexOf(this));
    }).on('other', function () {
      //...
    });

  players.push(player);

  res.json({ id: player.id });
});

//trigger the change of the name in another part of the code
app.put('/player/:id/name', function (req, res) {
  player.setName(req.body.name, function (err) {
    if (err){
      console.log(err);
      return res.send(500);
    }
    res.send(200);
  });
});

In this case the setName function has a callback but also emits an event that I need to handle in another part of the code.