patternjavascriptMinor
Optimize jquery plugin "on" method extension
Viewed 0 times
methodextensionpluginjqueryoptimize
Problem
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
jsFiddle to test
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);Solution
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:
-
This
would have been better served with
-
There is no point in declaring
just go for
From a once over style perspective:
function (key, element)
-
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
ifin$.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
Code Snippets
$.each(fctsToObserve, function (key, element) {
fctsObserveKeys += "hasChanged." + key + " ";
});observeMethods: function (namespace) {
var namespace = namespace ? "." + namespace : "";observeMethods: function (namespace) {
namespace = namespace ? "." + namespace : "";Context
StackExchange Code Review Q#25035, answer score: 2
Revisions (0)
No revisions yet.