The chatbot for The Community Post Pimping Board

I recently wrote a chat bot for a chat room here at Code Review. The chatroom is meant to be a place where people can come and pimp a post.

That chat bot is meant to handle a list of subscribed people and a list of pimped IDs.

-
People on the subscribed list can pimp posts and will get notified when someone else pimps a post in the chat room.

-
Post IDs on the list are posts that have already been pimped that day. A post cannot be pimped twice in one day.

```
var chat = document.getElementById("chat");
var input = document.getElementById("input");
var send = document.getElementById("sayit-button");
var subscribedStorage = "SIRPYTHON_PIMPBOT_SUBSCRIBED";
var pimpedStorage = "SIRPYTHON_PIMPBOT_PIMPED";

sessionStorage.setItem(pimpedStorage, "{}"); // so I don't get an undefined thing later

/**
Returns the last chat message spoken
*/
function getLastMessage() {
return {
content: chat.lastElementChild.children[1].lastElementChild.children[1].innerHTML,
user: chat.lastElementChild.children[0].children[2].innerHTML.replace(/ /g,'')
};
}

// ---------- Chat functions ----------
/**
Sends a message to chat
*/
function sendMessage(message) {
input.value = message;
send.click();
}
/**
Sends a message @ a user
*/
function sendTo(message, user) {
sendMessage("@" + user + " " + message);
}
// ---------- Subscribed list functions ----------
/**
Returns an object representing the subscribed list from storage
*/
function getSubscribedList() {
return JSON.parse(localStorage.getItem(subscribedStorage));
}
/**
Sets an object representing the subscribed list to storage
*/
function setSubscribedList(newList) {
localStorage.setItem(subscribedStorage, JSON.stringify(newList));
}
/**
Adds a user to the subscribed list
*/
function addToSubscribed(username) {
var subscribed = getSubscribedList();
subscribed[username] = true;
setSubscribedList(subscribed);
}
/**
Removes a user from the subscribed list

There are a few points that surely can be improved:

• Since you are checking if id == undefined, I recommend you to wrap your code in an anonymous function.

Why is that? Try this: var a, undefined=5; alert(a == undefined);. It will show false.

• You repeat this a lot:

if(isSubscribed(message.user) == true) {

if(wasPimped(id) == false) {

if(isSubscribed(message.user) == false) {


I don't think you really need to compare it with false or true.

• You have the following for loop:

for(var i = 0; i

You really should reduce the load a bit here.

Consider the following loop:

for(var i = 0, length = subscribed.length; i

This reduces the time because you don't have to access the length property everytime.

• You created a main function, but, where is it called?

I couldn't find anywhere where it is called, only when you pass it to setTimeout.

• Your main can be changed to make it more easy to understand what is going on.

Let's take the block to pimp a user:

if(isSubscribed(message.user) == true) {
    var id = messageParts[1];
    if(id == undefined) {
        sendTo("You need to provide the ID of your answer.", message.user);
    }

    if(wasPimped(id) == false) {
        addToPimped(id);
        var groupMessage = "";
        var subscribed = getSubscribedUsers();
        for(var i = 0; i < subscribed.length; i++) {
            groupMessage += ("@" + subscribed[i] + " ");
        }
        sendMessage(groupMessage);
        window.setTimeout(function() {
            sendMessage("http://codereview.stackexchange.com/a/" + id);
        }, 4000); // to prevent the chat from blocking the message due to it being sent too early
    } else {
        sendTo("That post has already been pimped today.", message.user);
    }
} else {
    sendTo("You must be subscribed to pimp here.", message.user);
}

Yeah, it's confusing. It took me 5 minutes to understand what is required to be pimped.

Consider the following code:

if( !isSubscribed(message.user) ) {
    sendTo("You must be subscribed to pimp here.", message.user);
} else {
    var id = messageParts[1];

    if( !id ) {
        sendTo("You need to provide the ID of your answer.", message.user);
    }
    else if( wasPimped(id) ) {
        sendTo("That post has already been pimped today.", message.user);
    }
    else {
        addToPimped(id);
        var groupMessage = "";
        var subscribed = getSubscribedUsers();
        for(var i = 0, length = subscribed.length; i < length; i++) {
            groupMessage += ("@" + subscribed[i] + " ");
        }
        sendMessage(groupMessage);
        window.setTimeout(function() {
            sendMessage("http://codereview.stackexchange.com/a/" + id);
        }, 4000); // to prevent the chat from blocking the message due to it being sent too early
    }
}

Each condition is in a logical order and easy to understand. You know what you need to do to pimp someone.

StackExchange Code Review Q#95708, answer score: 8