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

General feedback on utility module

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
feedbackmoduleutilitygeneral

Problem

I'm looking for general feedback on how to make this useful to other people. This is a subset of underscore.js with some additions, and what I feel are improvements.

The code passes jslint and minifies well with clousre.

Please let me know what questions I can answer to receive a better review.

```
/*****/
(function (self, undef) {

"use strict";

// holds (P)ublic properties

var $P = {},

// holds p(R)ivate properties

$R = {},

// native methods (alphabetical order)

nativeFilter = Array.prototype.filter,
nativeIsArray = Array.isArray,
nativeSlice = Array.prototype.slice,
nativeSome = Array.prototype.some,
nativeToString = Object.prototype.toString;

/****/
// GLOBAL MANAGEMENT

$P.noConflict = (function () {

// g is the single global variable

$R.g = '$';
$R.previous = self[$R.g];
$P.molist = {
utility: true
};
return function () {
var temp = self[$R.g];
self[$R.g] = $R.previous;
return temp;
};
}());

/****/
// TYPE CHECKS

$P.isType = function (type, obj) {
return $P.getType(obj) === type;
};

// returns type in captialized string form

$P.getType = function (obj) {
return nativeToString.call(obj).slice(8, -1);
};

$P.isFalse = function (obj) {
return obj === false;
};

$P.isUndefined = function (obj) {
return obj === undef;
};

$P.isNull = function (obj) {
return obj === null;
};

// detects null, and undefined

$P.isGone = function (obj) {
return obj == null;
};

// detects null, undefined, NaN, ('' ""), 0, -0, false

$P.isFalsy = function (obj)

Solution

-
When declaring variables, I suggest going with the var for each approach. Helps you in avoiding missing commas. Also, it's easier to read and tell they are variables especially when there are comments in between the set.

-
Name your variables verbosely. It can help when debugging and development. You wouldn't want to scroll up and back just to see what $P was. Name them like what they are. You can setup a non-verbose version like how jQuery references jQuery to $.

-
Don't mind the variable name length during the creation of the code. In the end, you'd still be using a minifier to shrink everything to single character variable names.

-
I suggest your getType be in lowercase to be consistent with the native typeof.

-
isFalse, isNull and isGone wouldn't be much of use. A direct comparison would be quicker to do and wouldn't cause that overhead of calling a function.

-
Do note that boolean promitives (true and false) are entirely different from boolean objects.

-
Make comments self explanatory. A sample of this scenario is where one asks "What does isObjectAbstract and isArrayAbstract? There are no abstracts in JS." - and the comments don't really explain much.

-
You seem to do type checks on some functions but others not.

-
Instead of returning false on failed type checks, I suggest you throw an error instead. Return the result when it succeeded, false if it didn't (but did use the function properly), and throw an error if something is not right. Remember console errors that go like "foo is not a function" or "Accessing property bar of undefined", same idea.

-
Before you do regexp on stuff, try doing it without using regexp. In eachString, I assume you are testing for a string. You can just do a typeof str === 'string' && str.length instead to check if it is a non-empty string.

-
I don't see the purpose of runTest. It seems to just require a name, an array and a function that receives the array. Also, there's a potential memory "leak" here. Calling runTest stores the function's results in tests, however, test isn't accessible outside the closure. The objects and it's contents will stay there, continually accumulating, normally inaccessible indefinitely.

Well, that's it. That's my rundown of the things in your code.

Context

StackExchange Code Review Q#30189, answer score: 5

Revisions (0)

No revisions yet.