"The 2nd Monitor" chatroom translator

Question

For those who go to The 2nd Monitor chatroom, you already know how bad I am at remembering everything. And, sometimes, the new visitors will wonder what something means.

For that, I've developed a very simple chat translator.

;(function(window, undefined){
    'use strict';

    var memes = {
        'zombie': {
            'title': 'Unanswered question (or with answers without upvotes)',
            'href': 'http://meta.codereview.stackexchange.com/a/1511/',
            'find': new RegExp('(zombie)','gi')
        },
        'Jamalized': {
            'title': 'Being Jamalized means that Jamal edited your question/answer',
            'href': 'http://meta.codereview.stackexchange.com/a/1675/',
            'find': new RegExp('(jamalized)','gi')
        },
        'TS': {
            'title': 'Theoretical Star (star it and say "RSA")',
            'href': 'http://meta.codereview.stackexchange.com/a/1526/',
            'find': new RegExp('(TS)','g')
        },
        'RSA': {
            'title': 'Real Star Applied (you say it after staring a message with "TS")',
            'href': 'http://meta.codereview.stackexchange.com/a/1526/',
            'find': new RegExp('(RSA)','g')
        },
        'Thanks, Santa!': {
            'title': 'When someone upvotes a post, and you don\'t know who, just say this',
            'href': 'http://meta.codereview.stackexchange.com/a/1526/'
        },
        'IWNPFETTOLAI': {
            'title': 'I will not provide further explanation than this overly long acronym itself',
            'href': 'http://meta.codereview.stackexchange.com/a/1673/'
        },
        'Monking': {
            'title': 'A greeting to the Monkey doing his monkey-business',
            'href': 'http://meta.codereview.stackexchange.com/a/1678/',
            'find': new RegExp('(monk(?:ing|ernoon|evening|night))','gi')
        },
        'TTQW': {
            'title': 'Time To Quit Work',
            'href': 'http://meta.codereview.stackexchange.com/a/1643/'
        },
        'TTGH': {
            'title': 'Time To Go Home',
            'href': 'http://meta.codereview.stackexchange.com/a/1643/'
        },
        'TTGTB': {
            'title': 'Time To Go To Bed',
            'href': 'http://meta.codereview.stackexchange.com/a/1643/'
        },
        'STM': {
            'title': 'Smoking The Documentation',
            'href': 'http://meta.codereview.stackexchange.com/a/1953/'
        },
        'overengineering': {
            'title': 'A case of when something has become overly-engineered',
            'href': 'http://meta.codereview.stackexchange.com/a/2520/'
        },
        'HOLY CARP': {
            'title': 'Probably HOLY CRAP misspelled',
            'href': 'http://meta.codereview.stackexchange.com/a/4928/'
        },
        'Malachi\'d': {
            'title': 'Something amusing or entertaining that hasn\'t been starred yet',
            'href': 'http://meta.codereview.stackexchange.com/a/1667/'
        },
        'JDQ': {
            'title': 'JavaDeveloper Question',
            'href': 'http://meta.codereview.stackexchange.com/a/2053/'
        }
    },
    translate = function(){
        var messages = document.querySelectorAll('#chat .message:not([data-checked="1"])'),
            fragment = document.createDocumentFragment(),
            tmp_content = document.createElement('div');

        if( messages && messages.length )
        {
            for(var i = 0, l = messages.length; i < l; i++)
            {
                var message = messages[i];

                message.setAttribute('data-checked', '1');

                var content = message.querySelector('.content');

                if( content && !content.firstChild.className || content.firstChild.className.indexOf('onebox') === -1 )
                {
                    tmp_content.innerHTML = content.innerHTML;
                    for(var meme in memes)
                    {
                        var html = '';
                        for(var j = 0, m = tmp_content.childNodes.length; j < m; j++)
                        {
                            //text nodes only
                            if( tmp_content.childNodes[j].nodeType === 3 )
                            {
                                if( memes[meme].find )
                                {
                                    html += tmp_content
                                        .childNodes[j]
                                        .nodeValue
                                        .replace(memes[meme].find, '<a href="' + 
                                            ( memes[meme].href || '#' ) +
                                            '" target="_blank" title="' +
                                            ( memes[meme].title || '' ) +
                                            '" style="color:inherit;border-bottom:1px dashed #000">$1</a>'
                                        );
                                }
                                else
                                {
                                    html += tmp_content
                                        .childNodes[j]
                                        .nodeValue
                                        .replace(meme, '<a href="' + 
                                            ( memes[meme].href || '#' ) +
                                            '" target="_blank" title="' +
                                            ( memes[meme].title || '' ) +
                                            '" style="color:inherit;border-bottom:1px dashed #000">'+
                                            meme+
                                            '</a>'
                                        );
                                }
                            }
                            else if( tmp_content.childNodes[j].nodeType !== 8 )
                            {
                                html += tmp_content.childNodes[j].outerHTML;
                            }
                        }
                        tmp_content.innerHTML = html;
                    }
                    content.innerHTML = tmp_content.innerHTML;
                }
            }
        }
    }

    translate();

    setInterval(translate, 5000);

})(Function('return this')());

The idea is that the chat will be augmented, in a certain way, to add meanings to certain words or sentences.
Those will have a link and a descriptive title.

To run it, simply fire the console. Every 5 seconds, the chat will be scraped in search of new messages.

In terms of clarity and content, what can I change or improve?

Answer 1

Be careful when using setInterval:

setInterval(translate, 5000);

If for some reason something started to hang up, maybe your browser or your client's connection to the server, a thread will stall, and it will cause other threads to stack up because one is created every 5 seconds.

It would be safer to move this line to the end of your translate function:

setTimeout(translate, 5000);

This is safer because this ensures that the translate function will only be called again after the first one has been completed.

---

You have a few magic numbers in your code:

tmp_content.childNodes[j].nodeType !== 8

and

tmp_content.childNodes[j].nodeType === 3

You should create "constants" for these places so they are more clear. A constant would look something like this:

var CONSTANT_NAME = constant_value;

And, although you don't say it, if you happen to use ES6 in the future for this code, you could use the const keyword, rather than the var keyword:

const CONSTANT_NAME = constant_value;

---

At one point in your code, when you are doing some HTML substitution, you do this:

.replace(meme, '<a href="' + 
    ( memes[meme].href || '#' ) +
    '" target="_blank" title="' +
    ( memes[meme].title || '' ) +
    '" style="color:inherit;border-bottom:1px dashed #000">'+
    meme+
    '</a>'
 );

Why do you bother with these parts:

( memes[meme].href || '#' )

and

( memes[meme].title || '' )

All memes have a both a title and a href, so you do you check of it's existence? This seems unnecessary and you should just remove them from this chunk (and the next one).

---

These two chunks are very similar:

html += tmp_content
    .childNodes[j]
    .nodeValue
    .replace(meme, '<a href="' + 
        memes[meme].href+
        '" target="_blank" title="' +
        memes[meme].title +
        '" style="color:inherit;border-bottom:1px dashed #000">'+
        meme+
        '</a>'
     );

and

html += tmp_content
    .childNodes[j]
    .nodeValue
    .replace(memes[meme].find, '<a href="' + 
        memes[meme].href +
        '" target="_blank" title="' +
        memes[meme].title+
        '" style="color:inherit;border-bottom:1px dashed #000">$1</a>'
    );

Note: in these chunks, I included the change from my last recommendation.

The only differences are:

• The first argument of .replace.

• The innerHTML of the a tag.

You should build the string that you are going to be .replaceing before you enter these sections, so you can then make those minor changes. And, you should also store the string that is being .replaced since that is also the same for both conditional cases.

Here is what I mean:

var htmlStr = '<a href="' + 
            memes[meme].href +
            '" target="_blank" title="' +
            memes[meme].title+
            '" style="color:inherit;border-bottom:1px dashed #000">';
        ); // replacing the thing being replaced
var nodeValue = tmp_content.childNodes[j].nodeValue; // being replaced

if( memes[meme].find ) {
    html += nodeValue
        .replace(memes[meme].find, htmlStr + '$1</a>');

} else {
    html += nodeValue
        .replace(meme, htmlStr + meme + '</a>');
}

Answer 2

And here I go:

• Take advantage of the fact that SE sites use jQuery. This will trim down your DOM manipulation code to a high degree.

• We usually have a rule in our team that the most you should nest for conditions is 2. If you have 3 or more nested, then it's time you use a function.

• One problem I find problematic about nested if-else and other conditional blocks is that it's hard to trace what they changed. Try writing ala Functional Programming, where you call a function, pass in a value, and get a new value. Combine it with ternaries or hash maps and it ain't so bad.

• I suggest against "one var for all vars" as it becomes hard to determine when the vars end and the real code starts. For instance, I tried cleaning your code, but the indent is off at first glance, then discovered the "one var".

• Use "early return" if you happen to have if blocks with no else and nothing after it.

  function foo(){
    if(bar){
      // code at second level
    }
  }
  
  // is the same as
  
  function foo(){
    if(!bar) return;
    // code at first level
  }
  

Now my jQuery is a bit rusty, but I think your logic can be simplified to the following. It may not be complete, but you get the idea.

setInterval(function translate() {

  // Look for a child element that is not .onebox from .content that's under a non-checked ancestor .message in #chat
  // Phew! That's a long one!
  $('#chat .message:not([data-checked="1"]) .content > :not(.onebox)').each(function(index, content){

    // Mark the ancestor .message as checked.
    // We use attr() instead of .data() because we're using a selector to look, and data() doesn't alter attributes
    $(content).closest('.message').attr('data-checked', 1);

    var contentChildren = $(content).children();

    // Looking for text nodes with jQuery filter
    contentChildren.filter(function(index, element){

      return element.nodeType === 3;

    }).each(function(index, element){

      var html = $(element).text();

      $.each(memes, function(key, meme){
        html.replace(meme.find || meme, '<a href="' + meme.href + '" title="' + meme.title + '">' + meme + '</a>'); 
      });

      element.replaceWith(html);
    });
  });

}, 5000);

Answer 3

You're matching the regexes in a loop, giving it \$O(n m)\$ performance instead of \$O(m)\$ where \$n\$ is the number of regexes and \$m\$ is the length of the text being matched.

Any halfway decent regex engine can match an arbitrary number of regexes simultaneously and then tell you which one matched (a trivial extension to the way a|b is matched; I've done this in C); I am not familiar enough with JavaScript to know if it has one.

Possibly since these regexes are fairly simple, you could capture a fixed piece of text and then use that in a dict lookup?

Answer 4

in your for loop I noticed that you were creating an m variable to hold the length of the childNodes and that isn't really necessary

 for(var j = 0, m = tmp_content.childNodes.length; j < m; j++)
 {

you can just do this instead

for(var j = 0; j < tmp_content.childNodes.length; j++)

---

if you are trying to increase the speed of the loop, and not access the length every loop, then you should do it like this

var m = tmp_content.childNodes.length;
for (var j = 0; j < m; j++)

it's a little cleaner, but it will be quicker than what I originally did, maybe just as fast as what you were doing. the variable does have a slightly different scope though.