8

The Basics As you said, I think as each handler grows in size and the base handler adds more common features, the prototypes will pay off in spades. Honestly, the biggest readability issue for me is the require-and-instantiate style you're using. Is this common? I haven't seen it in any of the code for the tools I mentioned in my comment above. Compare ...


6

In node, throwing an error without catching it will kill the process. This seems to be the primary error handling mode (and based on this it seems you then rely on pm2 to restart the process). This means that if even one client manages to trigger an error in your code, every client will be disconnected. Fixing this would require rewriting a lot of the code ...


5

Server-side code Looks pretty good! However, as discussed in the comments, your server should probably be checking the game state to curtail cheating. As it is, it's pretty easy to "win" by fiddling with the client side code. The server should: Keep the canonical game state Only respect a single playTurn message from the player who's turn it is Determine ...


5

You can gain a lot of readability by formatting the code (every decent editor will do a passable job at this with a single keystroke) and extracting several small functions (refactoring). I'll start with this but only extract the reusable functions. Note: I assumed you can emit the initial connection count after binding the commands. If not, move the ...


4

Extract the common logic to a function, where the varying parts are parameters. Unless I'm missing something, this looks trivially easy to do: function setup_namespace(org_id) { var nsp = io.of('/nsp_bucket_' + org_id); nsp.on('connection', function(socket){ console.log('someone connected to namespace bucket ' + org_id); socket.on('...


3

A short review; Your database related functions should be separated, in my mind your socket methods should only do this: Normalize parameters your receive like with isCheckedNum Call a method to Create/Read/Update/Delete data (with a promise mechanism) Emit to rooms if succesful and needed Send back the appropriate HTTP response code with extra info if ...


3

You should only open one connection to mongodb. Otherwise if you have 10 users connected on your chat, you'll have 10 opened connections to mongodb. var mongo = require('mongodb').MongoClient; var sio = require('socket.io'); mongo.connect('mongodb://127.0.0.1/chat', function(err, db) { if(err) throw err; var messages = db.collection('messages'); ...


2

A fun piece of code, You clearly took the single comma separated var statement all the way, I would still group the related variables ( requires, module, server info, timeouts etc.): var express = require('express'), routes = require('./routes'), net = require('net'), events = require('events'), app = module.exports = express....


2

Interesting question, I would rewrite try{ var arr = JSON.parse(data); }catch(e){ callback(e, false); } callback(null, arr.uid); as try{ var arr = JSON.parse(data); callback(null, arr.uid); }catch(e){ callback(e, false); } Actually, I would probably go even a step further and write it as try{ callback(null, JSON.parse(data).uid)...


2

So you're really looking to do two things: Be an Emitter: emit a random number from a list of random numbers to socket clients every m seconds; and Be a Fetcher: fetch n random numbers and add them to the list so the Emitter can get them. I'm not sure this is a stream problem - if you were to fetch the numbers one at a time and then emit them, sure, a ...


2

It looks like you've written the code that already exists. this way multiple subscriptions for the same events can be avoided but all the interested React components can be notified on an event update Why would you want to avoid this? There is no overhead in a function call. In fact, this is exactly how Socket.io client works (based on Emitter component):...


2

Your client JavaScript indentation is very, very messed up. I can hardly understand what's going on. Some lines are missing an indentation, and some have an extra indentation, or even an extra space (which is extra very messed up, because you are leaving the alignment of a tab/indentation). Everything inside the very first function should be indented one ...


2

As @Brythan explained, your code is off topic as it does not do as told. So in this answer I will only be reviewing the part that works: the HTML. Way too much canvas I don't quite understand why you have a canvas for each square when you are merely writing a single character to that square. Why don't you create a 3 by 3 table? That way, everything is ...


2

Conditional line not in brackets There is an if statement in queryAPI() without brackets. This impacts readability greatly- especially if the code to be conditionally executed gets moved to the line below. It is best to always include brackets. I haven’t tested this with your code but you could simplify the if/else to a statement that uses short-circuit ...


2

Linting The code reads well. There is one convention you missed: use \s to indicate whitespace. const encode = name => { return name.replace(/ /g, "+"); }; const encode = name => { return name.replace(/\s/g, "+"); }; And the else below is redundant. if (await api.data[0]) return await api.data[0]; else return { name: [], quote: [], ...


2

You have a few flags (removed, responded) which are almost equivalent and possibly useless. removeListener() won't cause any error if listener has been already removed then removed is unnecessary. After you called removeListener() your callback won't be called then also responded (which BTW is a misleading name) is unnecessary. Also, setTimeout() won't call ...


1

I'm not fantastic with Typescript; so let's take a look at your Python: Credentials app.config['SECRET_KEY'] = 'secret!' This shouldn't be baked into your code. It should be in a secure wallet of some kind. Resources on the internet abound about how to accomplish this, either in Python or at the operating system level. Business logic vs. presentation ...


1

Good comments from @Magnus Jeffs Tovslid that I would like to add to. It seems like you have at least three logical layers to the user experience. First, you have what you call the Room, which doesn't seem to have any "room" behaviors at all and whose relationship to Gamerooms is unclear. What is the function of the room? What is the meaning to the end ...


1

Just a few comments: 1: The Room class can quickly become a place to throw in all sorts of functionality, even when it's not needed by all its children. I would be careful to not put much there at all, and rather compose the different rooms with needed functionality. For example, a GameRoom may be composed by a chat, game queue, lobbies, and even user ...


1

In regards to the TODO comment, I would suggest something like this: const allowClickingRoomNameToJoin = () => { // TODO look for a better way to scope `this` with ES6 to eliminate `function()` $roomListSideNav.find('div').click(e => { chat.processCommand('/join ' + $(e.target).text()) $userMessageInputField.focus() }) } This removes ...


1

Your code looks quite clear, and I see only some possible improvements. Regarding your questions: I don't think it'd better to split into fetchRoomList and updateRoomList separate functions: there are a few common vars that should be computed twice; additionally it'd prevent increasing performance like I suggest below (by traversing HTML tree less times). ...


1

Not entirely sure why you'd keep calling connect and close. As far as my DB knowledge goes, you open a connection once and keep it open. Here's an answer from StackOverflow regarding the issue. So I suggest that once you get the db object, you simply just pass it on. In this case, simpleEmit should be handed over the db object. I recommend that you use ...


1

.bind should've been used here instead of .apply on: function(msg, callback) { socket.on(msg, $rootScope.$apply.bind($rootScope, callback.bind.apply(socket, arguments))); }, But seeing how complicated the shortened "one-liner" becomes, expanded form was probably better: on: function(msg, callback) { socket.on(msg, function(data){ $...


1

Just wanted to let you know quickly this: postData = postData.replace("/VIFE?", ""); function puts(error, stdout, stderr) { sys.puts(stdout) } exec('sudo bash /home/pi/Documents/Sketches/Bailey/server/' + postData, puts); This is incredibly dangerous, anybody with some some basic hacking knowledge can take complete control of your machine. I hope ...


1

From a once over: There is no need to set views or view engine for the server part, you are not using it Be careful about polluting the global namespace, variables like welcome, join and leaving should be declared with var I would probably call APPNAME -> APPNICK, this tells me that this will be used for chat messages From a memory perspective, you are ...


Only top voted, non community-wiki answers of a minimum length are eligible