Lorem ipsum chat bot

I'm a web development student, and I made a chat bot that emulates an SMS app commonly found on smartphones, etc. It responds with random lorem ipsum text. I'm wondering if my functions are too long and how/if I should refactor further. For instance, when building the chat bubbles, would using a DocumentFragment or cloneNode be better for my DOM manipulation?

It reads in a JSON object, which consists of an array of word lengths and words sorted by length (i.e. words[length][wordarray].)

Working example here.

```
/*
* Main module for the lorem ipsum chat, declares local vars, calls init() on
* load
*
*/

(function main() {
"use strict";

// Local vars
var words, // object for lorem ipsum JSON
xhr, // XMLHttpRequest object
chatInput, // chat input
chatHistory; // chat history window

/*
* init - initializes XMLHttpRequest, reads in the words object, and adds
* event listeners
*
*/
function init () {

// Get JSON using AJAX, parse to obj
xhr = getXHR();
xhr.open("get", "data/words.json", true);

xhr.send(null);
xhr.onreadystatechange = function() {
if (xhr.readyState === 4 && xhr.status === 200)
words = JSON.parse(xhr.responseText);
else
console.log("Ready state:" + xhr.readyState + " Status: " + xhr.status);
};

// initialize variables
chatInput = document.getElementById("chat");
chatInput.addEventListener("keyup", parseText, false);
chatHistory = document.getElementById("chat_history");
}

/**
* parseText is the callback for the keyup eventlistener, and listens for
* enter key to be pressed, signaling that the user has entered a message.
*
* @param {Event} event - keyup from chatInput
*
*/
function parseText(event) {
var message;

if (event.keyCode === 13 && chatInput.value) {

The design of your getXHR function is very confusing.

First off, by the top of the code, we learn that there is a variable xhr that is in a scope for all these functions to see.

Then, looking at this xhr function, if either of the first two conditionals pass, that outer xhr variable is going to be set, and then it will be returned.

Then, looking back at the init function, you are again storing the return of getXHR in the outer xhr variable.

Why are you doing it like this? This is very confusing. As a solution, I recommend that you get rid of the outer xhr variable because you only use it in init. Then, just handle the return from getXHR.

Since the outer xhr will no longer be used, here is what getXHR will look like:

function getXHR() {
    if (window.XMLHttpRequest) { // Mozilla, Safari, IE7+ ...
        return new XMLHttpRequest();
    }
    else if (window.ActiveXObject) { // IE 6 and older
        return new ActiveXObject("Microsoft.XMLHTTP");
    }
    else {
        window.alert("Your browser does not support AJAX.");
        return false;
    }
}

You don't do any handling of the return getXHR:

xhr = getXHR();
xhr.open("get", "data/words.json", true);

Looking at your getXHR function, it could possibly return false if it can't find a suitable XHR object to work with:

window.alert("Your browser does not support AJAX.");
return false;

Well, what if this function returns false? Your code is going to try to call all these functions that a normal XHR would have, but it would fail because that value is not an XHR.

You should add a conditional checking the return of this function. If the return is false, it should just exit the function immediately; this will stop everything else.

That giant else if chain in wordLengthByFrequency is very ugly. I recommend that you create an array.

In the array, the elements would be the random values to check. Then, if a check passes against an element, you can simply return the index of that element + 1.

That would look like this:

var rndm = Math.floor(Math.random() * 100);

var name = [5, 12, 21, 34, ...];

for(var i = 0, length = name.length; i < length; i++) {
    if(rndm <= name[i]) {
        return i + 1;
    }
}

DO NOT name the array "name"; I could not think of a good name for it.

StackExchange Code Review Q#101490, answer score: 3