Simple Telnet chat server

Question

I am new to node.js and asynchronous programming and this is my first small project to learn it. I have written a small Telnet chat server and the program works but I want to know if this is the correct way to write programs in node.js (asynchronous programming model).

var tcp = require('net');
var Server = tcp.createServer();
var pwd=[]; // keep tracks of connected clients 
var ip=[];  // keeps tracks of connected clients IP address
var Cpwd = "Rushabh"; // password that you require to login

Server.on('connection'  ,function(conn){
    conn.setEncoding('utf-8');
    conn.write("Password:");// ask user for password on their terminal
    console.log("[" + conn.remoteAddress + "] has joined the chat");

    conn.on('data' , function(data){
        if(pwd.indexOf(conn)>-1){//check if it is an old client or a new client
            console.log("[" + conn.remoteAddress + "]:"  + data); 

            if(pwd.length > 0){ // check if atleast one client is connected
                sendMessage(conn , data);// broadcast message to all client connected 
            }
        }
        else{//if it is a new client then server should first check for password 
            data= data.toString('utf-8').trim();
            var message = " has joined the chat";
            if(Cpwd == data){ // if it is a new client than check for password
                pwd.push(conn); 
                ip.push(conn.remoteAddress);
                sendMessage(conn , message);
            }
            else {
                conn.write("Password rejected:" + data);conn.end();}// disconnect client 
            }
        }); 

      conn.on('end' , function() { // remove the client from reference array

       var i , client;
         for(i in pwd){
           client = pwd[i];
           if(!client.writable){
             pwd.splice(i,1);
             console.log(ip[i]+ " has left the chat");
             ip.splice(i,1);
           }
         }

     });

}); 

    function sendMessage(conn , message){ //function to send message to all connected client

      var i , client;
      for(i in pwd){
        client = pwd[i];
        if(client.writable){     
          if(conn === client){       
            client.write("[me]:" + message);         
          }      
          else
            client.write("[" + conn.remoteAddress +"]:" + message);
        }
        else{
          pwd.splice(i , 1);
          console.log(ip[i]+ " has left the chat");
          ip.splice(i,1);
        }   
      }
    }
    Server.listen(8000);

Answer 1

From a once over:

Naming

• variable names should start with a lower case letter, unless they are constructors
  • Server -> server
  • Cpwd -> password
• pwd contains all the connections, any reader would guess it contains passwords

More on variables

• There is no good reason to have a separate array ip, since it always contains as many entries as pwd which already contains the the ip addresses in remoteAddress.
• Having the password inside your scrip is ok for a prototype, it is not for production

Functions

You encapsulated sendMessage into a function, you should also have a leave or quit function and a joinChat function.

Flow

• You announce that a user joins the chat before checking the password
• In on 'end' you could just use indexOf on conn instead of manually looping over all connections.

Other than that I think you are doing fine with the asynchronous model of Node.