0
\$\begingroup\$

Compare to underscorejs if it pleases you. Once again, I hope this is well commented. Please let me know what comments/improvements I can add.

Please review all aspects of this code.

/***************************************************************************************************
**UTILITY
 - additional coverage for looping, type checking, extending ...
 - consistent ordering, naming conventions ...
 - fewer function branches
 - increased type checking and modularity
 - vector based unit testing
***************************************************************************************************/

// self used to hold client or server side global ( window or exports )
(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;

    $P.noConflict = (function () {

        // $R.g holds the single global variable, used to hold all packages
        // methods
        $R.g = '$A';
        $R.previous = self[$R.g];

        // utility is required by all other packages
        // start the "pack"age list
        $P.pack = {
            utility: true
        };

        return function () {
            var temp = self[$R.g];
            self[$R.g] = $R.previous;
            return temp;
        };
    }());

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

    // returns type in a captialized string form
    // typeof is only accurate for function, string, number, boolean, and
    // undefined.  null and array are both incorrectly reported as object
    $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;
    };

    $P.isNumber = function (value) {
        return (typeof value === 'number') && isFinite(value);
    };

    // detects null or undefined
    $P.isGone = function (obj) {
        return obj == null;
    };

    // detects anything but null or undefined
    $P.isHere = function (obj) {
        return obj != null;
    };

    // detects null, undefined, NaN, ('' ""), 0, -0, false
    $P.isFalsy = function (obj) {
        return !obj;
    };

    // detects any thing but null, undefined, NaN, ('' ""), 0, -0, false
    $P.isTruthy = function (obj) {
        return !!obj;
    };

    // shortcut as their are only two primitive boolean values
    // detects a "boxed" boolean as well
    $P.isBoolean = function (obj) {
        return obj === true || obj === false ||
            nativeToString.call(obj) === '[object Boolean]';
    };

    // delegates to native
    $P.isArray = nativeIsArray || function (obj) {
        return nativeToString.call(obj) === '[object Array]';
    };

    // jslint prefers {}.constructor(obj) over Object(obj)
    $P.isObjectAbstract = function (obj) {
        // return obj === Object(obj);
        return !!(obj && (obj === {}.constructor(obj)));
    };

    // has a numeric length property
    $P.isArrayAbstract = function (obj) {
        return !!(obj && obj.length === +obj.length);
    };

    $P.someIndex = function (arr, func, con) {
        var ind,
            len;

        // prevent type errors, note function is validated
        if ((arr == null) || (arr.length !== +arr.length) || (typeof func !== 'function')) {
            return;
        }

        // delegate to native some()
        if (nativeSome && arr.some === nativeSome) {
            return arr.some(func, con);
        }

        // if the function passes back a truthy value, the loop will terminate
        for (ind = 0, len = arr.length; ind < len; ind++) {
            if (func.call(con, arr[ind], ind, arr)) {
                return true;
            }
        }
        return false;
    };

    $P.someKey = function (obj, func, con) {
        var key;

        // prevent type errors
        // for-in will filter out null and undefined
        if ((obj == null) || (typeof func !== 'function')) {
            return;
        }
        for (key in obj) {
            if (obj.hasOwnProperty(key)) {

                // if the function passes back a truthy value,
                // the loop will terminate
                if (func.call(con, obj[key], key, obj)) {
                    return true;
                }
            }
        }
        return false;
    };

    // loop through space separated "tokens" in a string
    $P.eachString = function (str, func, con) {

        // prevent type errors
        if (typeof str !== 'string' || str === "" ||
                typeof func !== 'function') {
            return;
        }
        $P.someIndex(str.split(/\s+/), func, con);
    };

    // does not extend through the prototype chain
    // implemented for objects only
    $P.extend = function (obj) {

        // loop througth elements beyond obj
        $P.someIndex(nativeSlice.call(arguments, 1), function (val) {

            // extend it
            $P.someKey(val, function (val_inner, key) {
                obj[key] = val_inner;
            });
        });
        return obj;
    };

    // extends non-prototype properties from obj2 on to obj1
    $P.extendSafe = function (obj1, obj2) {
        var key;
        for (key in obj2) {
            if (obj2.hasOwnProperty(key)) {
                if (obj1.hasOwnProperty(key)) {
                    throw "naming collision: " + key;
                }
                obj1[key] = obj2[key];
            }
        }
        return obj1;
    };

    // if incorrect types are passed, it will return an empty array
    // arrays only
    $P.filter = function (arr, func, con) {
        var results = [];
        if ((arr == null) || (typeof func !== 'function')) {
            return results;
        }
        if (nativeFilter && arr.filter === nativeFilter) {
            return arr.filter(func, con);
        }
        $P.someIndex(arr, function (val, ind, arr) {
            if (func.call(con, val, ind, arr)) {
                results.push(val);
            }
        });
        return results;
    };

    $P.clone = function (obj) {
        return $P.extend({}, obj);
    };

    $P.someIndex(['Arguments', 'Function', 'String', 'Number',
        'Date', 'RegExp', 'Object'], function (val) {
        $P['is' + val] = function (obj) {
            return $P.isType(val, obj);
        };
    });

    $P.addU = function (str, suf) {
        return str + '_' + suf;
    };

    $P.removeU = function (str) {
        if (typeof str !== 'string' || str === "") {
            return false;
        }
        return str.slice(str, str.lastIndexOf("_"));
    };

    $P.removeSuffix = function (str, len) {
        if (typeof str !== 'string' || str === "") {
            return false;
        }
        return str.slice(0, -len);
    };

    // equivalent to IIFE but "nicer" syntax
    $P.runTest = (function () {
        var tests = {};
        return function (name, arr, func) {
            tests[name] = func.apply(this, arr);
        };
    }());

    self[$R.g] = $P.extendSafe($P, {});

// this will hold the global object - window or exports
}(this));

someIndex() and someKey() compared to underscore each()

So the way I chose to write someIndex() and someKey() was organically or as a process. I wanted to abstract the looping idioms I got tired of writing into methods. Because for and for-in can be broke out of I decided that some() was a better building block then each() from my perspective. After, I did this I compared what I had to underscore. My design choices were different in other ways as well. -- Please note, if you need speed just use a for or for-in loop, if you need clean concise code, use the abstraction -- The other way I diverged from underscore is that I wrote some separately for arrays and objects, because when I code I always know which one I am looping through, hence I wanted to express this knowledge in my code - someIndex() and someKey(). Another way I diverged was that I type check all the inputs in one line, for some reason underscore chooses not to type check the callback function. The other way I diverged is I don't check for any internal breaks, that are internal use only, in underscore this is shown as an equality check for breaker. Also, as in all my code b.c. JS is not block scoped yet, I put my var definitions up top.

Also, regarding elegance, I feel my code is more elegant, as the type checking is consistent and consolidated, my var declarations are more naturally inline with how the interpreter reads them, and comments, white space, and a 4 space indent level make the code easier to read.

That's the gist of it, please let me know if I can elaborate more.

Below is the code for both:

    $P.someIndex = function (arr, func, con) {
        var ind,
            len;

        // prevent type errors, note function is validated
        if ((arr == null) || (arr.length !== +arr.length) || (typeof func !== 'function')) {
            return;
        }

        // delegate to native some()
        if (nativeSome && arr.some === nativeSome) {
            return arr.some(func, con);
        }

        // if the function passes back a truthy value, the loop will terminate
        for (ind = 0, len = arr.length; ind < len; ind++) {
            if (func.call(con, arr[ind], ind, arr)) {
                return true;
            }
        }
        return false;
    };

    var each = _.each = _.forEach = function(obj, iterator, context) {
    if (obj == null) return;
    if (nativeForEach && obj.forEach === nativeForEach) {
      obj.forEach(iterator, context);
    } else if (obj.length === +obj.length) {
      for (var i = 0, length = obj.length; i < length; i++) {
        if (iterator.call(context, obj[i], i, obj) === breaker) return;
      }
    } else {
      var keys = _.keys(obj);
      for (var i = 0, length = keys.length; i < length; i++) {
        if (iterator.call(context, obj[keys[i]], keys[i], obj) === breaker) return;
      }
    }
  };
\$\endgroup\$
3
\$\begingroup\$

The code itself looks somewhat clean, but I'd say it lacks the elegancy of Underscore.

Your (P)ublic / P(r)ivate convention is very weird; I can't train my eyes to tell $R from $P quickly.
Did you invent it yourself? Why do you use it at all? Private functions are hidden by scope anyway.

Don't throw strings for errors, throw new Error(message) instead. Errors preserve stack trace.

Also, I'd say the design goal isn't clear.

Underscore positions itself as the minimal toolset for manipulating JS basic types (objects, arrays, functions). Lo-dash positions itself as Underscore with options (e.g. it has deep cloning)—some like it, some think it does too much and lacks Underscore's simplicity.

These are two very different approaches, and different developers choose different libs.

Where does your library stand on this spectrum? Is this a learning project? On one hand, you have “basic” Underscorish functions like isArray and extend, but then you also offer arguably exotic functions like eachString, someKey or addU. They are hardly “minimal toolset”, and honestly, I don't understand what they do.

So what was your design goal?

\$\endgroup\$
3
\$\begingroup\$

I don't use underscore, so I am more or less naive about what you're meaning to accomplish and how it might look otherwise. Instead, from a consumer perspective, I am comparing/contrasting this utility with not having it only.

I am assuming "all aspects" includes code quality from the perspective of maintaining, reading, or extending the code as well, and I'll start there.

Reading this is made somewhat more challenging due to the similar-looking $P and $R. I believe this has been mentioned by another reviewer, but I would like to offer some workarounds. I assume that you do need an object for $R - if you don't, you should follow a previous reviewer's advice and be rid of it - to loop over its keys or use it as a prototype or dump it on the console at some point. It's okay to provide an abbreviated global name to consumers ($A for your library), but it doesn't pay to do it in your own scope. Even pub and priv are more easily distinguishable, immediately clear in their purpose, and if you count the shift key, it only costs you one keystroke per typed pair.

Speaking of saving and losing keystrokes, foo === false actually requires fewer keystrokes than $A.isFalse(foo). The same goes for isUndefined, isNull, isHere, and isGone. I've seen some other libraries do stuff like that, so you're not alone in this, but I've never found a use for any of it. That goes double, though, for isTruthy and isFalsy. I can't imagine wanting to spell that out. Just curious - why is there an explicit spell-it-out test function for everything but true? I guess if I was trying to maintain code written by real neanderthals that I didn't trust at all, it would be reassuring to see what they were intending to test explicitly in every conditional. In any case, it's more bondage & discipline than utility, and you can tell by how little use you get out of these functions throughout the rest of the code.

There are lots of opportunities to use those functions. There are many cases where the body of those isFoo functions as well as getType and isType appear repeated verbatim, with precisely the same intent. Normally, I would say it's overkill to call repeated simple boolean expressions a failure to DRY, but that's the point of your library, right? You wrote those functions because they're useful code, so use them.

The function isNumber returns false for boxed numbers. If there's a reason for that, you should probably add a comment about it and document it somewhere, or someone might assume that if $A.getType(n) === 'Number' then $A.isNumber(n) === true, since if $A.getType(b) === 'Boolean' then $A.isBoolean(b) === true.

I would rather see Object.keys(obj).forEach in someKey instead of using native for-in and filtering with hasOwnProperty. It takes up less vertical space, restricts the scope of key to the block that should be using it, and does the very same thing. I can read what you have, and it works, though. Just a matter of preference.

If I understand it correctly, the difference between Array.prototype.forEach and your someIndex is that the iteration of someIndex can be ceased from within the callback by returning truth-y. I would then expect, when I read code that uses someIndex and not forEach, that you are scanning an array to find something and intend to stop at some point. However, you go on to use the some functions in extend and filter where the intent of the loop is to iterate over all keys or array elements. It does show that you have a useful function, but it also reads a little hack-y. Each time I read it, I spend a second or two looking for a return to determine what you're looking for, only to realize that you just aren't using forEach.

I can't really comment on all the suffixes and underscores because, if they belong in the utility at all, they're tools specific to the rest of the project. Seeing slice, tokenizing, and meaningful underscores with no regex or grammar in sight makes me a little nervous (long story, code-based parsers are bad) but I have no idea what you're really using it for.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy