4
\$\begingroup\$

I would like feedback on how to optimize this jQuery plugin used for triggering change event on any DOM element. This plugin doesn't use a timer to check for DOM changes but instead extends some jQuery native functions. I'm mostly concerned by the .on() method extension because I'm sure there is better way of doing this.

jsFiddle to test

;(function ($) {
    var fctsToObserve = {
        append: [$.fn.append, 'self'],
        prepend: [$.fn.prepend, 'self'],
        remove: [$.fn.remove, 'parent'],
        before: [$.fn.before, 'parent'],
        after: [$.fn.after, 'parent']
    }, fctsObserveKeys = '';
    $.each(fctsToObserve, function (key, element) {
        fctsObserveKeys += "hasChanged." + key + " ";
    });
    var oOn = $.fn.on;
    $.fn.on = function () {
        if (arguments[0].indexOf('hasChanged') != -1) arguments[0] += " " + fctsObserveKeys;
        return oOn.apply(this, arguments);
    };
    $.fn.hasChanged = function (types, data, fn) {
        return this.on(fctsObserveKeys, types, null, data, fn);
    };
    $.extend($, {
        observeMethods: function (namespace) {
            var namespace = namespace ? "." + namespace : "";
            var _len = $.fn.length;
            delete $.fn.length;
            $.each(fctsToObserve, function (key) {
                var _pre = this;
                $.fn[key] = function () { 
                    var target = _pre[1] === 'self' ? this : this.parent(),
                        ret = _pre[0].apply(this, arguments);
                    target.trigger("hasChanged." + key + namespace, arguments);
                    return ret;
                };
            });
            $.fn.length = _len;
        }
    });
    $.observeMethods()
})(jQuery);
\$\endgroup\$
2
\$\begingroup\$

I dont know if there is a better way to do it, but it seems quite smart and minimally intrusive.

From a once over style perspective:

  • function (key, element) <- since you dont use element, you can remove it from the signature
  • This

    $.each(fctsToObserve, function (key, element) {
      fctsObserveKeys += "hasChanged." + key + " ";
    });
    

    would have been better served with Array.reduce, not sure if you want to support browsers that don't have Array.reduce

  • Naming.. Any code reviewer would be remiss if they did not point that you abbreviate your variables too much which makes the code needlessly harder to understand
  • In the if in $.fn.on = function () { you should use curly braces or a least a newline.
  • There is no point in declaring var namespace here:

    observeMethods: function (namespace) {
        var namespace = namespace ? "." + namespace : "";
    

    just go for

    observeMethods: function (namespace) {
        namespace = namespace ? "." + namespace : "";
    
  • Comments, some of the more trickier concepts could have used a line of comment
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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