Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This function's job is to replace several text smilies, e.g. :D, :), :love with the appropriate smiley image. In my opinion my code has several issues, yet the main problem is that a lot of quite similar regexes are created dynamically: one for each different smilie replaced. As you might expect, replacing smilies in a text is a job done kindly often, so this is indeed code that is executed a lot of time, therefore I think optimizing that makes sense.

Well, why didn't I just call text.replace()? I don't want the smilies to be replaced if they are enclosed by backticks: `

var replaceTextSmilies = function() {
    var smiliesShortcut = {};
    smiliesShortcut[':)'] = 'smile.png';
    /* adding much more */

    var smiliesWordy = {};
    smiliesWordy[':angry'] = 'angry.png';
    /* adding much more */



    function escapeRegExp(str) {
        return str.replace(/([.?*+^$[\]\\(){}|-])/g, "\\$1");
    }

    return function(text) {
        Object.keys(smiliesWordy).forEach(function(entry) {
            var image = '<img src="img/smilies/' + smiliesWordy[entry] + '"/>';

            if(text.indexOf(entry) !== -1) {
                var regex = new RegExp('('+ escapeRegExp(entry) +')(?=(?:(?:[^`]*`){2})*[^`]*$)', 'g');
                text = text.replace(regex, image);
            }
        });
        Object.keys(smiliesShortcut).forEach(function(entry) {
            var image = '<img src="img/smilies/' + smiliesShortcut[entry] + '"/>';

            if(text.indexOf(entry) !== -1) {
                var regex = new RegExp('('+ escapeRegExp(entry) +')(?=(?:(?:[^`]*`){2})*[^`]*$)', 'g');
                text = text.replace(regex, image);
            }
        });
        return text;
    };
}();
share|improve this question
add comment

1 Answer

You can store the regex as part of the smily metadata.

var replaceTextSmilies = function() {
    var smiliesShortcut = {};
    smiliesShortcut[':)'] = {
        img : 'smile.png'
    };
    /* adding much more */

    var smiliesWordy = {};
    smiliesWordy[':angry'] = {
        img : 'angry.png'
    };
    /* adding much more */

    addRegex(smiliesShortcut);
    addRegex(smiliesWordy);

    function escapeRegExp(str) {
        return str.replace(/([.?*+^$[\]\\(){}|-])/g, "\\$1");
    }

    function addRegex(smilies) {
        Object.keys(smilies).forEach(function(smily) {
            smilies[smily].regex = new RegExp('(' + escapeRegExp(smily) + ')(?=(?:(?:[^`]*`){2})*[^`]*$)', 'g');
        });
    }

    function replace(text, smilies) {
        Object.keys(smilies).forEach(function(entry) {
            var image = '<img src="img/smilies/' + smilies[entry].img + '"/>';

            if (text.indexOf(entry) !== -1) {
                text = text.replace(smilies[entry].regex, image);
            }
        });
        return text;
    }

    return function(text) {
        text = replace(text, smiliesWordy);
        text = replace(text, smiliesShortcut);
        return text;
    };
}();

Demo: Fiddle

Note: You can make it even better if you can manually create the regex and assign them instead of using addRegex() like

    smiliesShortcut[':)'] = {
        img : 'smile.png',
        regex: /<the regex>/
    };
share|improve this answer
 
Thank you, any other comments on the code? :) –  Michael Aug 13 '13 at 11:09
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.