12
\$\begingroup\$

Based on Node.js in Action (Manning Publications) chapter 2 "Building a Multi-room Chat Application", I've taken the model they presented (which is all typical ES5 and jQuery) and completely rewrote it using ES6 and adding a bit of functionality, as well as JSDoc throughout.

While the functionality is mostly identical (I added a bit of new functionality as well), the code is vastly different, and I would like to ask if it could be improved further.

This is what it looks like:

chat

In general, am I using some decent patterns, or are you seeing some trends in my structure, functions and objects that stinks?

I will present both the server (native Node.js along with socket.io for WebSocket) and client-side code (regular ES6 along with jQuery) with some brief explanation.


Server-side

./server.js

This server is responsible for receiving HTTP requests, serving the requested static assets (or an error), and starting the chat server (presented in next section).

"use strict"
const http = require('http')
const fileSystem = require('fs')
const path = require('path')
const mediaType = require("mime")
const chatServer = require('./lib/chat_server')

const fileCache = {}
const HTTP_SERVER_PORT = 3000
const HTTP_SUCCESS = 200
const HTTP_RESOURCE_NOT_FOUND = 404

/**
 * Respond when requested resource is OK.
 * @param {Object} response - the response from HTTP server
 * @param {string} filePath - where the requested file is
 * @param {Object} fileContents - the contents of the requested file
 */
const httpSuccess = (response, filePath, fileContents) => {
  const contentType = mediaType.lookup(path.basename(filePath))
  response.writeHead(HTTP_SUCCESS, { 'Content-Type': contentType})
  response.end(fileContents)
}

/**
 * Respond when the requested resource is not found.
 * @param {Object} response
 */
const httpResourceNotFound = response => {
  response.writeHead(HTTP_RESOURCE_NOT_FOUND, { 'Content-Type': 'text/plain' })
  response.write('Error ' + HTTP_RESOURCE_NOT_FOUND + ': ' + 'resource not found')
  response.end()
}

/**
 * Serve static files from server.
 * @param {Object} response
 * @param {string} absPath
 */
const httpServeStaticFile = (response, absPath) => {
  if (fileCache[absPath]) {
    httpSuccess(response, absPath, fileCache[absPath])
  }
  else {
    fileSystem.exists(absPath, exists => {
      if (exists) {
        fileSystem.readFile(absPath, (err, data) => {
          if (err) {
            httpResourceNotFound(response)
          }
          else {
            fileCache[absPath] = data
            httpSuccess(response, absPath, data)
          }
        })
      }
      else {
        httpResourceNotFound(response)
      }
    })
  }
}

/**
 * Get absolute file path from URL
 * @param {string} url
 * @returns {string}
 */
const getAbsolutePathFromUrl = url => {
  let filePath
  if (url === '/') {
    filePath = 'public/index.html'
  }
  else {
    filePath = 'public' + url
  }
  return './' + filePath
}

const httpServer = http.createServer((request, response) => {
  const absPath = getAbsolutePathFromUrl(request.url)
  httpServeStaticFile(response, absPath)
})

/**
 * Start listening with the HTTP server and the chat server.
 */
const startServers = () => {
  httpServer.listen(HTTP_SERVER_PORT, () => {
    console.log("Server listening on port " + HTTP_SERVER_PORT + ".")
  })
  chatServer.listen(httpServer)
}

startServers()

./lib/chat_server.js

This code handles all the WebSocket interactions on the server. It has a number of listeners/handlers as well as emitters, all of which are documented. I think overall it is pretty simple.

In the original code from the book, all the functions were included in exports.listen and I thought that was quite messy, so I extracted all the various code into their own distinct functions and only included the calls in exports.listen, which I think makes it much more maintainable and easier to document.

"use strict"
const socketIo = require('socket.io')

let io
const nickNames = {}
const namesUsed = []
let guestNumber = 1
const currentRoom = {}

/**
 * Start the Socket.IO server.
 * @param {Object} server - a running Node.js HTTP server
 */
exports.listen = server => {
  /* start Socket.IO server, allowing it
  to piggyback on existing HTTP server */
  io = socketIo.listen(server)
  io.set('log level', 1)
  /* define how each user connection will be handled */
  io.sockets.on('connection', socket => {
    const EMIT_TO = 'rooms'
    guestNumber = assignGuestName(socket, guestNumber, nickNames, namesUsed)
    handleGetCurrentUserNickname(socket)
    joinDefaultRoom(socket)
    handleMessageBroadcasting(socket)
    handleNameChangeAttempts(socket, nickNames, namesUsed)
    handleRoomJoining(socket)
    handleClientDisconnection(socket)
    socket.on(EMIT_TO, () => {
      socket.emit(EMIT_TO, io.sockets.manager.rooms)
    })
  })
}

/**
 * Emit user's current nickname.
 * @param {Object} socket - the user's client connection
 */
const handleGetCurrentUserNickname = socket => {
  socket.on('getCurrentUserNickname', () => {
    socket.emit('currentUserNameResult', {
      nickName: nickNames[socket.id]
    })
  })
}

/**
 * Assign user a guest name.
 * @param {Object} socket - the user's client connection
 * @param {Number} guestNumber - the global current/last guest number assigned
 * @param {Object} nickNames - the global list of connection IDs and their nicknames
 * @param {Array} namesUsed - the global list of all nicknames used
 * @returns {Number} - the guestNumber incremented by 1
 */
const assignGuestName = (socket, guestNumber, nickNames, namesUsed) => {
  const name = 'Guest' + guestNumber
  nickNames[socket.id] = name
  socket.emit('nameResult', {
    success: true,
    name: name
  })
  namesUsed.push(name)
  return guestNumber + 1
}

/**
 * Make user join the default room.
 * @param {Object} socket - the user's client connection
 */
const joinDefaultRoom = socket => {
  joinRoom(socket, 'Lobby')
}

/**
 * Join a chat room and notify the user and other users that a room was joined.
 * @param {Object} socket - the user's client connection
 * @param {string} room - the name of the room to join
 */
const joinRoom = (socket, room) => {
  socket.join(room)
  currentRoom[socket.id] = room
  socket.emit('joinResult', { room: room })
  socket.broadcast.to(room).emit('message', {
    text: nickNames[socket.id] + ' has joined ' + room + '.'
  })
  informOfUsersInRoom(socket, room)
}

/**
 * Inform user joining a room of the users present in the room.
 * @param {Object} socket - the user's client connection
 * @param {string} room - the name of the room to join
 */
const informOfUsersInRoom =(socket,room) => {
  const usersInRoom = io.sockets.clients(room)
  if (usersInRoom.length > 1) {
    let usersInRoomSummary = 'Users currently in ' + room + ': '
    for (let i in usersInRoom) {
      const userSocketId = usersInRoom[i].id
      if (userSocketId != socket.id) {
        if (i > 0) {
          usersInRoomSummary += ', '
        }
        usersInRoomSummary += nickNames[userSocketId]
      }
    }
    usersInRoomSummary += '.'
    socket.emit('message', { text: usersInRoomSummary })
  }
}

/**
 * Handle name-change attempts.
 * @param {Object} socket - the user's client connection
 * @param {Object} nickNames - the global list of connection IDs and their nicknames
 * @param {Array} namesUsed - the global list of all nicknames used
 */
const handleNameChangeAttempts = (socket, nickNames, namesUsed) => {
  socket.on('nameAttempt', (name) => {
    if (name.indexOf('Guest') == 0) {
      socket.emit('nameResult', {
        success: false,
        message: 'Names cannot begin with "Guest".'
      })
    }
    else {
      if (namesUsed.indexOf(name) == -1) {
        var previousName = nickNames[socket.id]
        var previousNameIndex = namesUsed.indexOf(previousName)
        namesUsed.push(name)
        nickNames[socket.id] = name
        delete namesUsed[previousNameIndex]
        socket.emit('nameResult', {
          success: true,
          name: name
        })
        socket.broadcast.to(currentRoom[socket.id]).emit('message', {
          text: previousName + ' is now known as ' + name + '.'
        })
      }
      else {
        socket.emit('nameResult', {
          success: false,
          message: 'Name "' + name + '" is already in use.'
        })
      }
    }
  })
}

/**
 * Send chat messages.
 * @param {Object} socket - the user's client connection
 */
const handleMessageBroadcasting = socket => {
  socket.on('message', message => {
    socket.broadcast.to(message.room).emit('message', {
      text: nickNames[socket.id] + ': ' + message.text
    })
  })
}

/**
 * Joins a room if it already exists, or creates it and joins if it doesn't.
 * @param {Object} socket - the user's client connection
 */
const handleRoomJoining = socket => {
  socket.on('join', room => {
    socket.leave(currentRoom[socket.id])
    joinRoom(socket, room.newRoom)
  })
}

/**
 * Clean-up after a user disconnects.
 * @param {Object} socket - the user's client connection
 */
const handleClientDisconnection = socket => {
  socket.on('disconnect', () => {
    const nickName = nickNames[socket.id]
    const nameIndex = namesUsed.indexOf(nickName)
    delete namesUsed[nameIndex]
    delete nickNames[socket.id]
    socket.broadcast.to(currentRoom[socket.id]).emit('message', {
      text: nickName + ' has disconnected.'
    })
  })
}

Client-side

./public/index.html

Just for reference, this is the HTML that the client-side code interacts with:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Chat</title>
  <link rel="stylesheet" href="/css/style.css" />
</head>
<body>
  <div id="content">
    <div id="room-name-header"></div>
    <div id="room-list-side-nav"></div>
    <div id="room-messages"></div>
    <form id="user-message-send-form">
      <input id="user-message-input-field" />
      <input id="send-button" type="submit" value="Send" />
      <div id="help">
        Chat commands:
        <ul>
          <li>Change nickname: <code>/nick [nickname]</code></li>
          <li>Join/create room: <code>/join [room]</code></li>
        </ul>
      </div>
    </form>
  </div>

  <script src="/socket.io/socket.io.js" type="text/javascript"></script>
  <script src="http://code.jquery.com/jquery-1.8.0.min.js" type="text/javascript"></script>
  <script src="/js/chat.js" type="text/javascript"></script>
  <script src="/js/chat_ui.js" type="text/javascript"></script>
</body>
</html>

./public/js/chat.js

This Chat class is a middle man of sorts between the chat_server and the jQuery GUI. Its methods use WebSocket to emit messages to the chat_server, while the jQuery parts (following this) handle most of the listening for WebSocket.

"use strict"

class Chat {
  /**
   * Handle the chat functionality.
   * @param {Object} socket - The socket assigned to the user's web client
   * @constructor
   */
  constructor(socket) {
    this.socket = socket
  }

  /**
   * Send message to a room.
   * @param {string} room - The room where the message is to be posted
   * @param {string} text - The message to be posted
   */
  sendMessage(room, text) {
    const message = {
      room: room,
      text: text
    }
    this.socket.emit('message', message)
  }

  /**
   * Get user's current nickname.
   */
  getCurrentUserNickname() {
    this.socket.emit('getCurrentUserNickname')
    this.socket.on('currentUserNameResult', response => {
      const nickName = response.nickName
      return nickName.toString()
    })
  }

  /**
   * Change the room the user is in.
   * @param {string} room - The name of the room to change to (i.e. join)
   */
  changeRoom(room) {
    const newRoom = { newRoom: room }
    this.socket.emit('join', newRoom)
  }

  /**
   * Handle processing of chat commands.
   * @param {string} command - A potential command string from user
   * @returns {string} - Whether the command was successful
   */
  processCommand(command) {
    const words = command.split(' ')
    // parse command from first word
    command = words[0]
      // account for the starting slash `/`
      .substring(1, words[0].length)
      .toLowerCase()
    let errorMessage = null
    switch(command) {
      case 'join':
        words.shift();
        const room = words.join(' ')
        this.changeRoom(room)
        break
      case 'nick':
        words.shift()
        const name = words.join(' ')
        this.socket.emit('nameAttempt', name)
        this.getCurrentUserNickname()
        break
      default:
        errorMessage = 'Unrecognized command: /' + command
        break
    }
    if (errorMessage) {
      return errorMessage
    }
  }
}

./public/js/chat_ui.js

This section of the original code was the typical tangled mess we've come to expect from jQuery. Thankfully, the large majority of it I was able to convert to use ES6 arrow functions, except for the function in the .click() event in allowClickingRoomNameToJoin() function (hence you will notice the TODO comment there; if you have any suggestions in that respect let me know).

Overall I think it reads a lot easier like this, I again extracted all those in-line functions to their respective functions to make them easier to document and maintain. This is my first experience with jQuery so I feel especially sure that this part of the code could be improved a lot still.

"use strict"

$(document).ready(() => {

  // jQuery selectors
  const $roomNameHeader = $('#room-name-header')
  const $roomListSideNav = $('#room-list-side-nav')
  const $roomMessages = $('#room-messages')
  const $userMessageSendForm = $('#user-message-send-form')
  const $userMessageInputField = $('#user-message-input-field')

  const socket = io.connect()
  const chat = new Chat(socket)

  /**
   * Run the main functionality of the client.
   */
  const clientMain = () => {
    socket.on('nameResult', displayNameChangeResults)
    socket.on('joinResult', displayRoomJoinResults)
    socket.on('message', displayReceivedMessage)
    socket.on('rooms', displayAvailableRoomsList)
    socket.on('currentUserNameResult', setNickname)

    let nickName = chat.getCurrentUserNickname()

    /**
     * Request list of rooms intermittently
     */
    setInterval(() => {
      socket.emit('rooms')
    }, 1000)

    $userMessageInputField.focus()

    /**
     * Allow submitting the form to send a chat message.
     */
    $userMessageSendForm.submit(() => {
      processUserInput(chat)
      return false
    })
  }

  const setNickname = result => {
    this.nickName = result.nickName
  }

  /**
   * Format user-submitted content to display in chat.
   * @param {string} message - The raw content from user
   * @returns {*|jQuery}
   */
  const divEscapedContentElement = message => {
    return $('<div class="user-message"></div>').text(message)
  }

  /**
   * Format system-submitted content to display in chat.
   * @param {string} message - The raw content from system
   * @returns {*|jQuery}
   */
  const divSystemContentElement = message => {
    return $('<div class="system-message"></div>').html(message)
  }

  /**
   * Route raw input to the correct channel.
   * @param {Object} chat - The applicable chat application
   */
  const processUserInput = chat => {
    const message = $userMessageInputField.val()
    if (isCommand(message)) {
      const systemMessage = chat.processCommand(message)
      if (systemMessage) {
        $roomMessages.append(divSystemContentElement(systemMessage))
      }
    }
    else {
      chat.sendMessage($roomNameHeader.text(), message)
      $roomMessages.append(divEscapedContentElement(nickName + ': ' + message))
      $roomMessages.scrollTop($roomMessages.prop('scrollHeight'))
    }
    $userMessageInputField.val('')
  }

  /**
   * Detect whether the given raw input is a command.
   * @param {string} message - The raw input
   * @returns {boolean} - Whether the raw input is considered a command
   */
  const isCommand = message => {
    return message.charAt(0) === '/'
  }

  /**
   * Display name changes.
   * @param {Object} result
   */
  const displayNameChangeResults = result => {
    let message
    if (result.success) {
      message = 'You are now known as ' + result.name + '.'
    }
    else {
      message = result.message
    }
    $roomMessages.append(divSystemContentElement(message))
  }

  /**
   * Display room joins.
   * @param {Object} result
   */
  const displayRoomJoinResults = result => {
    $roomNameHeader.text(result.room)
    $roomMessages.append(divSystemContentElement('Room changed'))
  }

  /**
   * Display a new message.
   * @param {string} message
   */
  const displayReceivedMessage = message => {
    const newMessage = $('<div></div>').text(message.text)
    $roomMessages.append(newMessage)
  }

  /**
   * Display available rooms.
   * @param {Array} rooms
   */
  const displayAvailableRoomsList = rooms => {
    $roomListSideNav.empty()
    for (let room in rooms) {
      room = room.substring(1, room.length)
      if (room !== '') {
        $roomListSideNav.append(divEscapedContentElement(room))
      }
    }
    allowClickingRoomNameToJoin()
  }

  /**
   * Allow clicking of a room name in room list to join said room.
   */
  const allowClickingRoomNameToJoin = () => {
    // TODO look for a better way to scope `this` with ES6 to eliminate `function()`
    $roomListSideNav.find('div').click(function() {
      chat.processCommand('/join ' + $(this).text())
      $userMessageInputField.focus()
    })
  }

  clientMain()

})
\$\endgroup\$
1
\$\begingroup\$

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 the anonymous function declaration and replaces it with an ES6+ arrow function as requested.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.