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

Game Server Querying

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

Problem

I made a package for Node.js that allowed the querying of game servers (or any UDP server(untested)) using UDP.

What it does is sends the query to the server and adds the query into a queue. When the message is received it retrieves the response from the server, parses it and removes the query from the queue.

Am I using the promises correctly and am I going about it the correct way?

Structure

  • lib



  • core.js



  • index.js



  • protocols



  • iw4



  • master.js



core.js

```
var Q = require('q'),
Class = require('extend.class'), //https://www.npmjs.org/package/extend.class
lookup = Q.denodeify(require('dns').lookup);

module.exports = Class.extend({
init: function() {
this._super();
this.options = {};
this.maxAttempts = 1;
this.attempt = 1;
this.finished = false;
this.udpTimeoutTimer = false;
},
start: function() {

var self = this;
return this.lookup()
.then(function() {
return self.run();
});

},
lookup: function() {
var deferred = Q.defer();

if (!('host' in this.options))
deferred.reject(new Error('No host specified'));

if (this.options.host.match(/^\d+\.\d+\.\d+\.\d+$/)) { //matched IP only
if (this.options.debug) {
console.log('IP matched: ' + this.options.host);
}
this.options.address = this.options.host;
deferred.resolve();
} else if (this.options.host.match(/^\d+\.\d+\.\d+\.\d+\:\d+$/)) { //matched IP:PORT, parse accordingly
var temp = this.options.host.split(':');
this.options.address = temp[0];
this.options.port = temp[1];
deferred.resolve();
} else { //host is a domain, look it up and retrieve the IP
var self = this;
return lookup(this.options.host).then(function(address) {
if (self.options.debug) {
console

Solution

Good code, I only have some minor pointers.

-
When I see this:

if (this.options.debug) {
    console.log('IP matched: ' + this.options.host);
}


in lookup and udpResponse, then I wonder why these functions have to know whether we are in debug mode. You should declare your intent to log by calling a single logging function. That function should know about debug mode and decide whether to actually log something.

  • I prefer to see comments prior to the statement, it preps the mind for what comes next, since code has a flow a->b->c->d you dont want to read a comment about a when you expect to be reading about b. Hope this makes sense, the coffee did not yet kick in.



  • 'use strict'; should be considered



  • There is so much this in cores.js.. Since you deal with a singleton, you might want to read up on the self revealing module pattern.



-
Do not hesitate to replace some if calls with ternaries:

if (this.options.parse) {
    this.deferred.resolve(this.parse(buffer));
} else {
    this.deferred.resolve(buffer);
}


could be

this.deferred.resolve( this.options.parse ? this.parse(buffer) : buffer );


  • In udpResponse I would call console.log before resolving. In general I shy away from doing anything after resolving.



  • Not a big fan of silent failure, I would have expected an else for if (len == 6) {, I also would have liked a comment for 6 because that is one mysterious magical constant.



Still, all in all good code IMO.

Code Snippets

if (this.options.debug) {
    console.log('IP matched: ' + this.options.host);
}
if (this.options.parse) {
    this.deferred.resolve(this.parse(buffer));
} else {
    this.deferred.resolve(buffer);
}
this.deferred.resolve( this.options.parse ? this.parse(buffer) : buffer );

Context

StackExchange Code Review Q#64272, answer score: 3

Revisions (0)

No revisions yet.