Node.js chat service

Question

I've built a simple chat server/client on Node.js and socket.io that I would like reviewed. My main concern is making the chat.js (client) running as cleanly as possible (OO) and streamlining data back and forth so I can add client monitoring (updated list of connected users) features down the line without issue.

Aside from that, I'm open to any critique/feedback about these sections of code on best practices or design patterns. This project is mostly for learning an unfamiliar technology (socket.io and, somewhat, node.js).

Here is the server code:

server.js:

var express = require('express');
var app = express();
var PORT = 7029;
var APPNAME = "Hermes";

//Set Views
app.set('views', __dirname + '/templates');
app.set('view engine', 'jade');
app.engine('jade', require('jade').__express);
app.get('/', function(req,res) {
    res.render('page');
});

//Setup server
app.use(express.static(__dirname + '/public'));
var io = require('socket.io').listen(app.listen(PORT));
io.set('transports', ['xhr-polling']);
console.log('Listening on port ' + PORT);

//Event Handlers
io.sockets.on('connection', function(socket) {
    welcome = new Message("Welcome to the ZeroDae chat service.", APPNAME);
    socket.emit('message', welcome );
    socket.on('send', function(data) {
        message = new Message(data.content, data.user);
        io.sockets.emit('message', message);
    });
    socket.on('join', function(data) {
        join = new Message(data + " has joined the chat.", APPNAME);
        io.sockets.emit('message', join);
    });
    socket.on('disconnect', function() {
        leaving = new Message("Someone has left.", APPNAME);
        io.sockets.emit('message', leaving);
    });
});



function Message(content, user) {
    this.content = content;
    this.user = user;
}

Here is the chat client:

chat.js:

$(document).ready(function () {
    var messages = [];
    var socket = io.connect('http://chat.zerodaedalus.com');
    var field = $('#field');
    var send = $('#send');
    var content = $('#content');
    var current = $('#current');
    var warning = $('#browserWarning');
    var user = getUser();
    var username = $('#username');
    var update = $('#update');
    var id;

    function User() {

    }

    //Hide warning
    warning.hide();

    //set username and display it on screen
    username.val(user);
    currentUser();

    //event handlers
    send.click(submit);
    update.click(setUser);
    field.keyup(function (e) {
        if(e.which === 13) {
            submit();
        } 
    });
    username.keyup(function (e) {
        if(e.which === 13) {
            setUser();
        } 
    });

    socket.on('message', function(data) {
        if(data.content) {
            if(data.user === 'Hermes')
                classes = 'message text-muted ';
            else
                classes = 'message ';
            content
                .append('<p></p>')
                .find('p:last-child')
                .addClass(classes + id)
                .append('<span></span>')
                .find('span:last-child')
                .addClass('user')
                .text(data.user + ': ')
                .closest('.message')
                .append('<span></span>')
                .find('span:last-child')
                .addClass('content')
                .text(data.content)
        }
        else {
            console.log('There is a problem:', data);
        }
    });

    function getUser() {
        if(typeof(Storage) !== "undefined") {
            if(localStorage.getItem('username'))
                name = localStorage.getItem('username');
            else
                name = generateUsername();
        }
        else {
            warning.show();
        }
        return name;
    }

    function setUser() {
        localStorage.setItem('username', username.val() );
        if(username.val() === '')
            user = generateUsername();
        else
            user = username.val();
        currentUser();
    }

    function generateUsername() {
        return 'zd-' + Math.floor((Math.random() * 729) + 1);
    }

    function currentUser() {
        current.html('&nbsp;(Currently: ' + user + ')');
        socket.emit('join', user);
        console.log(user);
    }

    function submit() {
        var text = $('#field').val();
        socket.emit('send', { content: text, user: user });
        field.val('');
        content.animate({scrollTop: content.prop('scrollHeight')});
    }
});

• GitHub
• Demo

Answer 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 creating a fresh set of functions on each connection. I would declare the 3 events handlers as functions outside of your connection handler, and then assign them

  function onSend(data) {
      message = new Message(data.content, data.user);
      io.sockets.emit('message', message);
  }
  
  function onJoin(data) {
      join = new Message(data + " has joined the chat.", APPNAME);
      io.sockets.emit('message', join);
  }
  
  function onDisconnect() {
      leaving = new Message("Someone has left.", APPNAME);
      io.sockets.emit('message', leaving);
  }
  
  //Event Handlers
  io.sockets.on('connection', function(socket) {
      welcome = new Message("Welcome to the ZeroDae chat service.", APPNAME);
      socket.emit('message', welcome );
      socket.on('send', onSend);
      socket.on('join', onJoin);
      socket.on('disconnect', onDisconnect);
  });
  

• From a readability and annoyance perspective I would modify Message so that the user gets defaulted to APPNAME or APPNICK

  function Message(content, user) {
     this.content = content;
     this.user = user || APPNICK;
  }
  

  This way you can simply

  function onJoin(data) {
      join = new Message(data + " has joined the chat." );
      io.sockets.emit('message', join);
  }
  

• Then from a DRY perspective, you are repeating io.sockets.emit('message', a ton of times. What if you made the sending of a Message a function of it ?

  Message.prototype.send = function(){
    io.sockets.emit('message', this);
  }
  

  Then your 3 handlers would look like this:

  function onSend(data) {
      new Message(data.content, data.user).send();
  }
  
  function onJoin(data) {
      new Message(data + " has joined the chat.").send();
  }
  
  function onDisconnect() {
      new Message("Someone has left.").send();
  }
  

  The next thing that would bother me is the repeat new, but I will stop here ;)

For the client part

• 13 should be a constant var ENTER = 13 with if(e.which === ENTER ) {

• This :

      content
          .append('<p></p>')
          .find('p:last-child')
          .addClass(classes + id)
          .append('<span></span>')
          .find('span:last-child')
          .addClass('user')
          .text(data.user + ': ')
          .closest('.message')
          .append('<span></span>')
          .find('span:last-child')
          .addClass('content')
          .text(data.content)
  

  looks impressive, but is definitely not the most performing piece of code, but it is fast enough, then I would leave it alone.

• console.log('There is a problem:', data); <- Problems happen with chat, I would make this part of the DOM, users will thank you for it

Fun piece of code, easy to follow.