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

BigBrother - A chat room watcher

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

Problem

Originally, this script stemmed from a small script that watched stars and who made them, but a few days after launching, SE patched it so the people behind the stars aren't sent.

Regardless, BigBrother, named after the character from 1984, had other functions, like monitoring obscene or unsightly chat messages and monitoring entering and exiting the room.

I was in a heavy rush while coding this, so it's a lot more spaghetti than I expected.

There's a few things you need to change manually to run this:

  • Username, so it can avoid reflagging obscene posts it shares



  • Chatroom id, on line 162. Currently set to The Sandbox



Here's the code on GitHub too.

If you want to run this code, comment out the lines relating to calling emit and it should run with posting messages in console only.

```
// ==UserScript==
// @name BigBrother
// @namespace github.com/The-Quill
// @version 0.1
// @description A chatroom watcher
// @author Quill
// @match http://chat.stackexchange.com/rooms/1/sandbox
// @grant none
// ==/UserScript==

(function (global) {
"use strict";

//username
var USERNAME = 'BigBrother';
var WELCOME_TEXT = 'initialising.';
//magic numbers
var EVENT_TYPES = {
MessagePosted: 1
, MessageEdited: 2
, UserEntered: 3
, UserLeft: 4
, RoomNameChanged: 5
, MessageStarred: 6
, DebugMessage: 7
, UserMentioned: 8
, MessageFlagged: 9
, MessageDeleted: 10
, FileAdded: 11
, ModeratorFlag: 12
, UserSettingsChanged: 13
, GlobalNotification: 14
, AccessLevelChanged: 15
, UserNotification: 16
, Invitation: 17
, MessageReply: 18
, MessageMovedOut: 19
, MessageMovedIn: 20
, TimeBreak: 21
, FeedTicker: 22
, UserSuspended: 29
, UserMerged: 30
// Custom Event Types
, Notable: 'notable'
, Welcome: 'welcome

Solution

Bug(s?) in createTaskPool

This function makes a reference to lastMessageIndex,
but I cannot find that anywhere in the code.
Perhaps it was meant to be lastTaskIndex instead?

function createTaskPool() {
    return setInterval(function() {
        if(tasks.length > 0) {
        // dequeue the message
        var lastTaskIndex = tasks.length - 1;
        var task = tasks[lastMessageIndex];
        tasks.splice(0, lastTaskIndex);
        executeTask(task);
        }
    });
}


Also the body of the if statement should be indented.

But even more importantly, I'm quite confused by this function.
Essentially it takes the last element from the array, then deletes all elements in front of the last element,
then execute the task.
Didn't you mean to delete the last element and keep everything else?
Writing the splice like this:

tasks.splice(lastTaskIndex);


Conditional logging

It seems the purpose of report is to decide to print messages with console.log or not.
Instead of having many if (report) ... conditions here and there,
it would be better to create a wrapper function to perform that check, for example:

function report(msg) {
    if (!settings.report) return;
    console.log(msg);
}


Repeated processing

In this code:

matchedContent = (body.match(KEY_WORDS[i].regex) == null ? 0 : body.match(KEY_WORDS[i].regex).length);


If the regex matched, then it will be performed again.
It would be better to execute body.match(KEY_WORDS[i].regex) once and cache its result.

Confusing logic in processMessage

In processMessage,
the scoring logic is strangely asymmetric for regex and plain strings:

if ('regex' in KEY_WORDS[i]){
    matchedContent = (body.match(KEY_WORDS[i].regex) == null ? 0 : body.match(KEY_WORDS[i].regex).length);
} else {
    matchedContent = (body.split(' ' + i + ' ').length === 1 ? 0 : body.split(i).length / 2)
}


When using regex, and there is a match,
since all your patterns use /mi flags,
it seems to me that the value of matchedContent will be always 1.

By contrast, when using plain strings,
the value will be roughly the number of occurrences / 2 + 0.5.
It's really hard to see the logic there.
Also note that the logic is case sensitive in this case,
and that seems undesirable.

To remedy this, I have two suggestions:

-
Change the regex flags to /gmi. With the /g flag, the value here becomes the number of occurrences instead of always 1.

  • Btw all the current regexes use /mi. If you will always use the same flags, then you might want to not specify the flags at all in the KEY_WORDS object, but add /gmi automatically always.



-
Convert plain strings to regex by wrapping between \b (word boundary), and add the /gmi flag too. This makes them case insensitive, like the regexes. After this change, you can continue processing like the regexes, so the scoring will be automatically consistent, as there will be one ultimate scoring system for both types (regex, plain strings)

Avoid returning two kinds of types

processMessage returns two kinds of types, numeric or boolean:

return sum >= 1 ? sum : false;


I suggest to stick with one kind of return type, numeric in this case.

Avoid pointless statements

This is ugly:

matchedContent > 0 ? console.log([matchedContent, 'instances of', i].join(' ')) : '';


When matchedContent <= 0 this will execute the pointless statement '';.
It would be better to rewrite this as an if statement.

if (matchedContent > 0) console.log([matchedContent, 'instances of', i].join(' '));

Code Snippets

function createTaskPool() {
    return setInterval(function() {
        if(tasks.length > 0) {
        // dequeue the message
        var lastTaskIndex = tasks.length - 1;
        var task = tasks[lastMessageIndex];
        tasks.splice(0, lastTaskIndex);
        executeTask(task);
        }
    });
}
tasks.splice(lastTaskIndex);
function report(msg) {
    if (!settings.report) return;
    console.log(msg);
}
matchedContent = (body.match(KEY_WORDS[i].regex) == null ? 0 : body.match(KEY_WORDS[i].regex).length);
if ('regex' in KEY_WORDS[i]){
    matchedContent = (body.match(KEY_WORDS[i].regex) == null ? 0 : body.match(KEY_WORDS[i].regex).length);
} else {
    matchedContent = (body.split(' ' + i + ' ').length === 1 ? 0 : body.split(i).length / 2)
}

Context

StackExchange Code Review Q#115033, answer score: 8

Revisions (0)

No revisions yet.