HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Am I including these JavaScript files correctly?

Submitted by: @import:stackexchange-codereview··
0
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) .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:

  • Do not abbreviate: isfn -> isFunction, isstr -> isString, cbpropcp -> ? ,slc -> slice, _requiregcloadcb -> ? etc. etc. etc. etc.



  • Also, use lowerCamelCase



  • Also, avoid _xx for 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 declare e as a parameter



  • You use cbrmprop and 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.