patternjavascriptMinor
Am I including these JavaScript files correctly?
Viewed 0 times
thesejavascriptfilesincludingcorrectly
Problem
I just want to know if this is reasonably good implementation of including local/remote JavaScript files.
The function takes a dictionary with (string)
// #includejs
;((function (name, def) {
this[name] = def(document);
}).call(
this, "includejs", function (doc) {
// will be used to reference private includejs() version
var _include;
// holds cached script urls
var imported = {};
// no-op callback default
var pass = function () {};
// helpers
var _ = {
// helper element to resolve urls to absolute
anchor: doc.createElement("a"),
// calculates difference of two arrays
// used by includejs() to filter new urls to load
arrdiff: function (a1, a2) {
return a1.filter(_.cbuniq).filter(_.cbdiff, a2);
},
// calculates intersection of two arrays
// used by includejs.reload() to filter cached urls
arrinters: function (a1, a2) {
return a1.filter(_.cbuniq).filter(_.cbinters, a2);
},
cbdiff: function (node) {
//this: a2[]
return this.indexOf(node) == -1;
},
cbinters: function (node) {
//this: a2[]
return this.indexOf(node) != -1;
},
cbpropcp: function (name) {
//this: {src{}, target{}}
this.target[name] = this.src[name];
},
cbrmprop: function (name) {
//this: target{}
try {
delete this[name];
} catch (e) {}
},
cbuniq: function (node, idx, arr) {
return idx reference for temporarily injecting -s
h: doc.getElementsByTagName("head")[0],
// attaches properties of (object) items to (object) target
// used in few plac
The function takes a dictionary with (string)
.src whitespace-separated list of urls to include/execute in global context, and (func) .done/.error/.load callbacks to run in corresponding cases, resolves URLs to absolute, temporarily inserts ` blocks in page header, caches loaded addresses, and attaches few static properties: (object) .defaults and (func) .ls/.reload to main includejs() function.
``// #includejs
;((function (name, def) {
this[name] = def(document);
}).call(
this, "includejs", function (doc) {
// will be used to reference private includejs() version
var _include;
// holds cached script urls
var imported = {};
// no-op callback default
var pass = function () {};
// helpers
var _ = {
// helper element to resolve urls to absolute
anchor: doc.createElement("a"),
// calculates difference of two arrays
// used by includejs() to filter new urls to load
arrdiff: function (a1, a2) {
return a1.filter(_.cbuniq).filter(_.cbdiff, a2);
},
// calculates intersection of two arrays
// used by includejs.reload() to filter cached urls
arrinters: function (a1, a2) {
return a1.filter(_.cbuniq).filter(_.cbinters, a2);
},
cbdiff: function (node) {
//this: a2[]
return this.indexOf(node) == -1;
},
cbinters: function (node) {
//this: a2[]
return this.indexOf(node) != -1;
},
cbpropcp: function (name) {
//this: {src{}, target{}}
this.target[name] = this.src[name];
},
cbrmprop: function (name) {
//this: target{}
try {
delete this[name];
} catch (e) {}
},
cbuniq: function (node, idx, arr) {
return idx reference for temporarily injecting -s
h: doc.getElementsByTagName("head")[0],
// attaches properties of (object) items to (object) target
// used in few plac
Solution
Your code is interesting,
you are clearly smart and know JavaScript, but this is a maintenance nightmare. I am assuming you will not take my advice to heart but here goes:
-
Name functions for what they do, not how they are used:
you are clearly smart and know JavaScript, but this is a maintenance nightmare. I am assuming you will not take my advice to heart but here goes:
- Do not abbreviate:
isfn->isFunction,isstr->isString,cbpropcp-> ? ,slc->slice,_requiregcloadcb-> ? etc. etc. etc. etc.
- Also, use lowerCamelCase
- Also, avoid
_xxfor private variables, as per Crockford
-
Name functions for what they do, not how they are used:
fnid: function (node) {
return node;
},fnid is a terrible name, if possible I would refactor the code so that I would not need this function. A better name might be value ?- JSHint could not find anything wrong, except that your event handlers do not use
e, so you do not need to declareeas a parameter
- You use
cbrmpropand similar functions only once, consider in-lining them
settup->setup;)
opts.s.splice(0, 1/0);->opts.s.splice();
defer : false,-> I think this should have been an option
Code Snippets
fnid: function (node) {
return node;
},Context
StackExchange Code Review Q#43803, answer score: 5
Revisions (0)
No revisions yet.