patternjavascriptMinor
Utility library and Underscore mixin - 1
Viewed 0 times
underscoreutilitymixinlibraryand
Problem
This code is similar to Underscore. I've added in some functions to fill in different use cases.
For example, one can use
Underscore does not have a good way to loop through
``
Pub.getType = function (obj) {
return nativeToString.call(obj).slice(8, -1);
};
Pub.isType = fun
For example, one can use
someKey to iterate through localStorage and sessionStorage.Underscore does not have a good way to loop through
localStorage / sessionStorage as it is incorrectly detected as an array like object, i.e. it has a length property but does not have indices.``
/*****
UTILITY
This is a small and efficient utility library. There is additional coverage,
consistent ordering, consistent naming conventions, increased input validation, increased structure,
and fewer function branches compared to underscore.
*/
/*jslint
browser: true,
forin: true,
plusplus: true,
eqeq: true,
ass: true
*/
(function (global, undef) {
"use strict";
// holds (Pub)lic properties for the package
var Pub = {},
// holds (Priv)ate properties for the package
Priv = {},
// native prototype methods
nativeSlice = Array.prototype.slice,
nativeSome = Array.prototype.some,
nativeToString = Object.prototype.toString;
// handles global variable management
Pub.noWar = (function () {
// Priv.g holds the single user-defined global variable
Priv.g = '$A';
Priv.previous = global[Priv.g];
Pub.pack = {
utility: true
};
return function () {
var temp = global[Priv.g];
global[Priv.g] = Priv.previous;
return temp;
};
}());
// returns type in a capitalized string form
// typeof is only accurate for function, string, number, boolean, and
// undefined. null and array are both reported as objects
// also typeof does not detect "boxed" values such as new Number(1)`Pub.getType = function (obj) {
return nativeToString.call(obj).slice(8, -1);
};
Pub.isType = fun
Solution
Quick review, in no particular order. I haven't gone line by line, I just scrolled around and noted what I saw.
Firstly, the good stuff:
Then, the not-so-good stuff
-
Naming convention may be internally consistent (not that I can really tell), but your library is not, in my opinion, quite large enough to get away with it. The first function I come across is called
It's almost ironic that a function that exists to play nice with other code goes out of its way to do things differently.
-
-
Similar to the above,
-
These two comments confuse me:
So... in one case, you use your own name for something, underscore be damned, but in the very next function, you "break naming convention" (though I don't see how) specifically to match underscore. Huh?
-
Naming in general: I just don't get many of these names, and with spotty documentation, I'm often left wondering. For instance
-
Speaking of
-
-
Furthermore it looks like a copy-paste job, since all the variables are
-
-
In all, this doesn't seem like a generic library (like underscore). It does have some generally applicable functions, sure, but it's also got some pretty context-specific and sometimes mystifying functions.
Firstly, the good stuff:
- Consistent style (almost, see #8 below)
"use strict";
- jslint-checked
Then, the not-so-good stuff
-
Naming convention may be internally consistent (not that I can really tell), but your library is not, in my opinion, quite large enough to get away with it. The first function I come across is called
noWar. This seems to be (the documentation is lacking) your library's implementation of the common noConflict function - so why not call it that? Right now it's just a cutesy name for a function that should aim for external consistency.It's almost ironic that a function that exists to play nice with other code goes out of its way to do things differently.
-
isGone and isFalsy should be defined as negations of calling isHere and isTruthy respectively. Or vice-versa. The whole point is that those functions report the opposite of their counterparts; don't write separate logic for either one, even when that logic is pretty straightforward.-
Similar to the above,
hasLength should also use isGone in its internal check. Don't repeat logic. And someIndex should be calling hasLength and isType in its checks. If your library doesn't trust its own functions, why should anyone?-
These two comments confuse me:
// *underscore calls this, isObject
...
// *breaks naming convention for compatibility with underscoreSo... in one case, you use your own name for something, underscore be damned, but in the very next function, you "break naming convention" (though I don't see how) specifically to match underscore. Huh?
-
Naming in general: I just don't get many of these names, and with spotty documentation, I'm often left wondering. For instance
testKeys. I can read the code, but I have no idea what I'd ever use it for.-
Speaking of
testKeys: Why does it take a callback when it's not asynchronous? It can just return a boolean. How can I be sure it'll do what I expect? Objects are unordered, so if the keys object contains the keys foo and bar, and my pattern string is "foobar", the string that's being tested might still be "barfoo". And it doesn't use hasOwnProperty like everything else.-
runTest doesn't appear to let anyone access its internal tests object, nor does it return the result of the test being run. So you run a test, and... what? You always get undefined back, and tests is forever a "private closure".-
prettyTime. Nice - unless you want another language than English, of course. The entire function just doesn't seem germane to whatever else the library is doing.Furthermore it looks like a copy-paste job, since all the variables are
snake_cased, while the rest of you code is camelCased. My bad, the style is consistent with the other functions (see comments). I would add, though, that distinguishing between "functions" and "variables" is sort of pointless in JS. Functions are variables. So I'm not sure the distinction makes sense.-
addU and removeU. These functions are just overly specific (and complex). Besides, removeU will fail if the original suffix contains an underscore already: removeU(addU("test", "my_suffix")) will return "test_my". So it'll only work for some kinds of strings, meaning it's probably built to work for your strings in your project. In another context, you or anyone else might have to do their own utility methods anyway, so as a utility library function there's not much value here, I'm afraid.-
firstKey. Again: Objects do not guarantee any ordering of their keys, so the function doesn't really do anything useful. At worst it misinforms the caller. It's entirely dependent on the vagaries of the runtime. To quote the spec: "The mechanics and order of enumerating the properties [...] is not specified." (emphasis added)In all, this doesn't seem like a generic library (like underscore). It does have some generally applicable functions, sure, but it's also got some pretty context-specific and sometimes mystifying functions.
Code Snippets
// *underscore calls this, isObject
...
// *breaks naming convention for compatibility with underscoreContext
StackExchange Code Review Q#64401, answer score: 4
Revisions (0)
No revisions yet.