HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Chat.SE in Terminal

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
chatterminalstackoverflow

Problem

This is a script that accesses the SE impromptu APIs to receive the fkey, a variable need for connection to chatrooms, and uses it to build a websockets connection which is handled by Node.js.

This is also available on GitHub.

It requires your username and password in the ws.js file and looks like this once executed:

chat.js:

```
var messages = {};
var stars = {};
var flagged = [];
var EVENT_TYPES = {
MessagePosted: 1
, MessageEdited: 2
, UserEntered: 3
, UserLeft: 4
, RoomNameChanged: 5
, MessageStarred: 6
, UserMentioned: 8
, MessageFlagged: 9
, MessageDeleted: 10
, UserNotification: 16
, Invitation: 17
, MessageReply: 18
, MessageMovedOut: 19
, MessageMovedIn: 20
};
function convert(str){
return !str ? "":
str.replace(/&/g, "&")
.replace(/>/g, ">")
.replace(/</g, " 25 ? message.substring(0, 12) + "..." : message) + "' to say: " + event.content,
event.room_name
);
} else {
say (
event.user_name + "edited a post to say: " + event.content,
event.room_name
)
}
messages[event.message_id] = event.content;
break;
case EVENT_TYPES.UserEntered:
say(event.user_name + " entered " + event.room_name);
break;
case EVENT_TYPES.UserLeft:
say(event.user_name + " left " + event.room_name);
break;
case EVENT_TYPES.RoomNameChanged:
say(event.user_name + " changed the name of " + event.room_name + " to " + event.content.substring(0, event.content.lastIndexOf(" /")));
break;
case EVENT_TYPES.MessageStarred:
event.message_id in stars ? event.message_id++ : event.message_id = 1;
if (messages[event.message_id] > 3){
say(
"highly starred message: " + event.content,
e

Solution

Very nice overall organization, use of promises, and use of constants where appropriate! Your code was a pleasure to read.

General comments:

  • You shouldn't implement convert(). It seems that you're receiving html from the WebSocket server. You should include a library to properly parse it and convert it to text. You already have cheerio, so you can use the .text() method to get just the raw text (with all the encoded entities properly converted). As a side suggestion, you could run this html through a HTML2Markdown converter so that the output doesn't lose the rich text formatting or the links



  • In general, it's probably a bad idea to hardcode a username and password into a script. I'd have it either passed in as a command line arg (with the password possibly entered interactively for security) which defaults to values stored in a dotfile in the user's home directory (something like ~/.sechatlogin)



  • As a matter of taste, and so that chat.js is more reusable, I'd forgo the console.log() in it and make chat into an event emitter. Then in ws.js you can listen for the specific events and output the proper text to stdout.



  • I see a lot of building URLs by concatenating strings. Since you can't be sure that some variables (which you receive from the server before concatenating into the URL) won't contain characters than need to be URL encoded, you're probably better off using a URL building library to handle this for you (look at url.format())



  • Since there are so many event types, it might be beneficial to extract their stringification logic into separate objects. So for each event create an object that knows how to print that event. Then, have chat.js convert a raw event object into a specialized event object depending on the type. This is a bit OO-ey, but it could be a nice abstraction if you need to use the events in other ways/places. The benefit of this, is that in ws.js you can just use chat.on('event', function(e) { console.log(e.toString()); }); to print events.



A few nitpicks:

  • You can define your constants with const instead of var



  • You should probably stick to ending all statements with semicolons or never ending statements with semicolons. I'd prefer the former, but whatever style you prefer pick one and stick to it!



  • There are a few mis-indented lines here and there



  • I would keep the number of hardcoded URLs to a minimum (try to read as many as you can from the returned content). This way your code is more future proof. For the ones you can't replace, turn them into constants with descriptive names instead of hardcoding them into the requests calls



  • LAST_EVENT should be camelCase since it changes

Context

StackExchange Code Review Q#116457, answer score: 9

Revisions (0)

No revisions yet.