1
\$\begingroup\$

I added more features to a Socket.IO tutorial and wanted to know how I can improve my code. I have some questions about how it's built at the moment, especially when a client or the server should handle a task.

The app has the following features:

  • Real-time Chat
  • Shows who is online
  • Custom client identification/nicknames

Aside from comments on my code, I'd like to ask:

  1. Given the online feature, which is implemented with an array pushed to the DOM after every dis/connection, is there a faster way to update who is online, rather than replacing everything?

  2. Would it be better to have the client keep track of its identification, or have the server do it? Right now the app has the server keep track, given a connected socket.

  3. Does the var count, online in the server's index.js count as bad practice, given they're technically global variables?

  4. Should I arrange the client's index.js event handlers and DOM event listeners differently? Should I group event handlers and event listeners together based on some criteria? Right now they're arranged by what they commonly deal with.

Here is the code for clients:

var socket = io();

document.getElementById('m').focus();

/* Messages */
document.getElementById('m-form').addEventListener('submit', function(event) {
    event.preventDefault();
    socket.emit('message', document.getElementById('m').value);
    document.getElementById('m').value = '';
});

socket.on('message', function(msg) {
    document.getElementById('messages').innerHTML += ('<li>' + msg + '</li>');
});

/* Identification */
socket.on('identification', function(id) {
    document.getElementById('identification-display').innerHTML = id;
});

socket.on('updateOnline', function(online) {
    document.getElementById('online').innerHTML = '';
    
    for(let i = 0; i < online.length; i++) {
        document.getElementById('online').innerHTML += `<li class="on">${online[i]}</li>`;
    }
})

// On new identification, update server and UI
document.getElementById('identification-form').addEventListener('submit', function(event) {
    event.preventDefault();

    let newID = document.getElementById('identification').value;
    socket.emit('setIdentification', newID);

    document.getElementById('identification').value = '';
    document.getElementById('identification-display').innerHTML = newID;
});
* { margin: 0; padding: 0; box-sizing: border-box; }
body { font: 13px Helvetica, Arial; }
header { background-color: rgb(130, 224, 255); padding: 10px}
#m-form { background: #000; padding: 3px; position: fixed; bottom: 0; width: 100%; }
#m-form input { border: 0; padding: 10px; width: 90%; margin-right: .5%; }
#m-form button { width: 9%; background: rgb(130, 224, 255); border: none; padding: 10px; }
#messages { list-style-type: none; margin: 0; padding: 0; }
#messages li { padding: 5px 10px; border-bottom: 1px solid black}
<!doctype html>
<html>
  <head>
    <title>Socket.IO chat</title>
    <link rel="stylesheet" href="main.css">
  </head>
  <body>
    <header>
      <h1>Chat</h1>
      <p>Identification: <span id="identification-display"></span></p>
      <form id="identification-form" action="">
        <input id="identification" autocomplete="off"><button>Set identification</button>
      </form>
      <ul id="online"></ul>
    </header>
    <ul id="messages"></ul>
    <form id="m-form" action="">
      <input id="m" autocomplete="off" /><button>Send</button>
    </form>
    <script src="/socket.io/socket.io.js"></script>
    <script src="index.js"></script>
  </body>
</html>

And the code for the NodeJS server:

var express = require('express');
var app = express();

var http = require('http').Server(app);
var io = require('socket.io')(http);

app.use(express.static('public'));

app.get('/', function(req, res) {
    res.sendFile(__dirname + '/public/index.html');
});

// Counter for connecting clients
var count = 0;
var online = [];

function updateOnline() {
    io.emit('updateOnline', online);
}

io.on('connection', function(socket) {
    /* Setup socket and notify clients */
    let identification = ++count;

    socket.emit('identification', identification);
    io.emit('message', `${identification} connected`);

    online.push(identification);
    updateOnline();

    /* Socket Events */
    socket.on('setIdentification', function(newIdentification) {
        io.emit('message', `${identification} changed their ID to ${newIdentification}`);
        let index = online.indexOf(identification);
        // Can this be improved with a map of socket: id?
        online[index] = newIdentification;
        identification = newIdentification;
        updateOnline();
    });

    socket.on('message', function(msg) {
        // Server handles the appending of identification
        msg = `${identification}: ${msg}`;
        io.emit('message', msg);
    });

    socket.on('disconnect', function() {
        io.emit('message', `${identification} left the server`);
        let index = online.indexOf(identification);
        // Can this be improved with a map of socket: id?
        online.splice(index, 1);
        updateOnline();
    });
    
});

http.listen(8000, function() {
    console.log('Listening on port 8080');
});

\$\endgroup\$

Your Answer

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

Browse other questions tagged or ask your own question.