current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite

I've written a jQuery Plugin which searches text & replaces it with other text or HTML.

Now everything works quite well but recently I came across this SO question where the OP could have use of my plugin. That's why I linked to the github repo & asked him for critics and/or contribution.

Soon after he said that my plugin would have slow down his page by the factor 17 (I don't think this number is coming from the dev console though). Anyway, that comment made me think about how to make my plugin faster.

So here's the code:

(function ($) {
    "use strict";
    $.fn.replaceMe = function (options) {
        var settings = $.extend({
            textToReplace: 'any_text',
            replaceWithText: '<span>othertext</span>',
            globally: true,
            excludedTags: [
                'img',
                'span'
            ]
        }, options);
        $(this).contents().each(function (index, node) {
            var text,
                excludedTags;
            if(node.innerHTML !== undefined) {
                text = node.innerHTML;
            } else {
                text = node.nodeValue;
            }
            excludedTags = (node.localName === null || node.localName === undefined) ? '' : node.localName;
            if (text.match(new RegExp(settings.textToReplace, 'g')) && node.nodeType === 3 && !excludedTags.match(new RegExp(settings.excludedTags.join('|')))) {
                if(settings.globally) {
                    $(node).replaceWith(text.replace(new RegExp(settings.textToReplace, 'g'), settings.replaceWithText));
                } else {
                    $(node).replaceWith(text.replace(new RegExp(settings.textToReplace), settings.replaceWithText));
                }
            }
        });
    }
}(jQuery));

On the testing environment, the text to parse counts 169'310 chars or 27'391 words.

The chrome console says that the script needs 42 ms to perform. Now I'm not sure if the script needs any improvement and if yes what could be done to make it faster.

share|improve this question
    
I would think a time complexity issue would come from the usage of Regex. – Quill Dec 11 '15 at 13:58
up vote 2 down vote accepted

I would think that a major time complexity comes from the usage of Regex which involves 'steps' in the processing of the expression, meaning the complex the statement, the more steps it can take.

Instead of using a Regex for when settings.globally is false, a normal String.replace can be done.

And instead of using a Regex for the global checks, I would suggest using a String.prototype.replaceAll polyfill like the following:

String.prototype.replaceAll = function(search, replace){
    var string = this;
    var index = 0;
    while ((index = string.indexOf(search)) !== -1) {
        string = string.replace(search, replace);
    }
    return string;
}

Which would change the settings.globally block into:

if(settings.globally) {
    $(node).replaceWith(text.replaceAll(settings.textToReplace, settings.replaceWithText));
} else {
    $(node).replaceWith(text.replace(settings.textToReplace, settings.replaceWithText));
}

I would also suggest removing jQuery from the solution as well. extend, $ (the element fetching), match and each can be polyfilled in, or simply converted to vanilla JavaScript.

function $(selector, el) {
    if (!el) { el = document; }
    return Array.prototype.slice.call(el.querySelectorAll(selector));
}
var extend = function(out) {
  out = out || {};
  for (var i = 1; i < arguments.length; i++) {
    if (!arguments[i])
      continue;
    for (var key in arguments[i]) {
      if (arguments[i].hasOwnProperty(key))
        out[key] = arguments[i][key];
    }
  }
  return out;
};
if (!String.prototype.includes) {
  String.prototype.includes = function() {'use strict';
    return String.prototype.indexOf.apply(this, arguments) !== -1;
  };
}

and for each: you can use Array.prototype.forEach (The polyfill is quite large, so I didn't include it) or a simple for loop.

share|improve this answer
    
Thank you for the answer. I'll test that out. I don't understand the last part of your answer "removing jQuery from the solution as well. extend, $ (the element fetching), match and each can be polyfilled in, or simply converted to vanilla JavaScript". Could you eventually explain that or link to some docs? – Daenu Dec 11 '15 at 15:21
    
Daenu, I've added polyfills for the referenced functions – Quill Dec 11 '15 at 15:43
    
Now I got it. Seems that I've to update my JS skills ;-) – Daenu Dec 11 '15 at 15:51

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.