2
\$\begingroup\$

I need some feedback for this set of value-test functions (are they doing what they say they do). Also ways to improve some of them, suggestions to add more test functions, etc.

// Object.test.isnumeric('0x12') -> true, etc.
!(( function ( field, define ) {
    this[field] = define();
} ).call(
Object,
"test",
function () {

    var _inner = {
        corstr:function ( o ) { return Object.prototype.toString.call( o ); },
        corePrimitiveTypes : ['[object Undefined]', '[object Null]', '[object Number]', '[object String]', '[object Boolean]'],
        emptyValues : [void 0, null, false, 0, ""],
        reg:{
            SCALAR     : /^boolean|number|string$/,
            URL        : /^(?:[a-z]+:\/\/)?(?:\/)?(?:\.\.\/)*[a-z][\w\-.]*(?:\:\d+)?[\/\w#!:.?+=&%@!\-]+$/i,
            EMAIL      : /[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?/,
            VALUE_TYPE : /\b(\w+)\]$/
        }
    };

    return {
        // #isbool
        isbool: function (o) {
            return _inner.corstr( o ) === '[object Boolean]';
        },
        // #isnum
        isnum: function (o) {
            return isFinite( o ) && _inner.corstr(o) === "[object Number]";
        },
        // #isnumeric
        isnumeric: function (o) {
            try {
                return isFinite( o = o["valueOf"]() )
                       && ( eval( o ) === parseFloat( Number( o ) ) );
            } catch(e) {}
                return false;
        },
        // #isunum
        isunum: function (n) {
            return this.isnum( n ) && ( n >= 0 );
        },
        // #isint
        isint: function ( n ) {
            return n === +n && isFinite( n ) && !( n%1 );
        },
        // #isuint
        isuint: function ( n ) {
            return this.isint( n ) && ( n >= 0 );
        },
        // #isfloat
        isfloat: function ( n ) {
            return +n === n && ( !isFinite(n) || !!( n % 1 ) );
        },
        // #isprimitive
        isprimitive: function ( o ) {
            return _inner.corePrimitiveTypes.indexOf( _inner.corstr( o ) ) != -1;
        },
        // #isstr
        isstr: function ( o ) {
            return _inner.corstr( o ) === "[object String]";
        },
        // #isfn
        isfn: function (o) {
            return typeof o === "function";
        },
        // #iswin
        iswin: function (o) {
            return o && o.top && ( o === o.window );
        },
        // #isarray
        isarray: function ( o ) {
            return this.isfn( Array.isArray )
                   ? Array.isArray( o )
                   : _inner.corstr( o ) === '[object Array]';
        },
        // #isarraylike
        isarraylike: function (o) {
            if ( !o ) return false;
            if ( this.iswin( o ) ) return false;
            var len = o.length,
                T   = this.type( o );
            if ( o.nodeType === 1 && len ) return true;
            return T === "array"    ||
                   T !== "string"   &&
                   T !== "function" &&
                   (
                    len === 0
                    || typeof len === "number"
                    && len > 0
                    && (len - 1) in Object( o )
                   );
        },
        // #isplainobj
        isplainobj: function (o) {
            return _inner.corstr( o ) === "[object Object]";
        },
        // #isemptyobj
        isemptyobj: function ( o ) {
            try {
                for ( var l in o )
                   return false;
               return true;
            } catch( e ) {
                return false;
            }
        },
        // #isdata , -> true for: [ {}+ ] structures
        isdata: function (o) {
            return  this.isarray( o )
                    && o.every(
                        function ( v ) {
                            return this.isplainobj( v );
                        },
                        this
                    );
        },
        // #isobj
        isobj: function (o) {
            return o === Object(o);
        },
        // #isscalar, -> true for: true/false, string, number
        isscalar: function ( o ) {
            return _inner.reg.SCALAR.test( this.type( o ) );
        },
        // #isempty, -> true for: undefined, null, 0, "", false, NaN, {}, []
        isempty: function ( o ) {
            if ( arguments.length === 0 )
                return void 0;
            var key, i, len, T;
            for (
                i = 0,
                len = _inner.emptyValues.length;
                i < len;
                i++
            ) {
                if (
                    o === _inner.emptyValues[i]
                    || o !== o
                ) return true;
            }
            if ( this.isarray( o ) ) return o.length == 0;
            if ( this.isobj( o ) ) return this.isemptyobj( o );
            return false;
        },
        // #isvalid, -> false, for: undefined, null, NaN
        isvalid: function ( o ) {
            return      o !== void 0
                   &&   o !== null
                   && ( o === o );
        },
        // #isdomobj
        isdomobj: function (el) {
            return this.isobj(el)
                   && ( el instanceof Node )
                   && (
                       ( el.ownerDocument || el )
                       .documentElement
                       .nodeName
                       .toUpperCase() === "HTML"
                    );
        },
        // #isurl
        isurl: function ( str ) {
            return _inner.reg.URL.test(  String( str )  );
        },
        // #isemail
        isemail: function (str) {
            return _inner.reg.EMAIL.test( String( str ) );
        },
        // #isxmlnode
        isxmlnode: function ( elm ) {
            var docElement =
                ( elm ? elm.ownerDocument || elm : 0 ).documentElement;
            return docElement
                   ? documentElement.nodeName !== "HTML"
                   : false;
        },
        // #ishtm
        ishtm: function ( input, _testel ) {
            return (
                      _testel = doc.createElement("div"),
                      _testel.innerHTML = String( input ),
                      _testel.getElementsByTagName("*").length > 0
                   );
        },
        // #type
        type : function ( o ) {
            if ( o === void 0 )
                return "undefined";
            var out;
            (
                ( out = this.iswin( o )
                      && 'window'
                      || _inner.corstr( o )
                        .match( _inner.reg.VALUE_TYPE )[1]
                        .toLowerCase()
                ) === 'number'
            )
            && (
                isFinite( o ) || ( out = String( o ) )
            );
            return out;
        }
    };
    //
}
));
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

There are always a lots of comments that can be made, but without doing an extremely deep analysis, here's what I could identity:

Global comments

1 - I see no reason to use such a complex module definition syntax.

The following is perfectly fine:

!function (obj, field) {
    //privates

    obj[field] = { ... };
}(Object, 'test');

2 - Stick to a single naming convention. You are sometimes using the camelCase convention, but your public interface is alllowercase (which is very hard to read in my opinion btw). The most widely spread convention in JavaScript is the camelCase one since that's the one used for native objects.

3 - When checking for null or undefined, you can simply rely on the type coersion of the == operator which will return true for null == undefined. Note that null == rightSideValue will only be true if rightSideValue is null or undefined, which makes this method safe to use.

4 - Whenever possible, I wouldn't rely on the string representation of the object to determine it's type. Using typeof is perfectly fine.

function isNumber(o) {
    return typeof o === 'number' && isFinite(o);
}

function isBool(o) {
    return typeof o === 'boolean';
}

//etc.

5 - You do not need to return void 0 to return undefined, simply use return;.

6 - It's slow to use an array as a lookup structure. Use a plain Object as a map instead.

var lookupMap = {
    a: true,
    b: true,
    c: true
};

'a' in lookupMap;

Instead of:

var lookup = ['a', 'b', 'c'];

lookup.indexOf('a') !== -1;

7 - There is no point to access an hard-coded property using the [] notation. o["valueOf"]() can just be o.valueOf().

8 - It isin't a good practice to add an extra argument just to declare a private variable without having to use the var x; syntax like you are doing in ishtm. It might not be harmful for the bahaviour but it is for code comprehension. Also it could lead to issues when using a documentation generator.

Looking at functions

isnumeric - I am not sure that it's actually better and you would have to test performances, however here's an alternative without relying on eval and parseFloat with try-catch.

return isFinite(o) && !(
    //the following are needed because isFinite uses Number(o) internally
    this.isArray(o)
    || this.isDate(o) 
    || this.isBoolean(o) 
    || (this.isString(o) && /^\s*$/.test(o))
    || o === null
);

However, if we take your path, cant you simplify as:

try {
    return isFinite(parseFloat(o));
} catch (e) {
    return false;
}

isemptyobj - I am not sure what implementation you want here, however keep in mind that the for in loop will iterate over inherited properties as well. You might want to consider checking if the object hasOwnProperty.

isplainobj - Again I am not sure what implementation you want here, but note that the following returns true.

Object.prototype.toString.call(new function A(){}) === '[object Object]'; //true

Perhaps you want this instead?

Object.getPrototypeOf({}) === Object.prototype); //true
Object.getPrototypeOf(new function A(){}) === Object.prototype; //false

Other ideas

A generic is function, something like (lets suppose it's part of the public API).

function is(o) {
    return [].slice.call(arguments, 1).some(function (type) {
        var fn = this['is' + type], rx;
        return fn? 
            fn.call(this, o) : 
            (rx = this.reg[type.toUpperCase()]) && rx.test(o);
    }, this);
}

You could call it like:

Object.test.is('test@test.com', 'email');

Object.test.is([], 'array', 'boolean', 'string');

Anyway, I could continue like this for a while, however without having your implementation's documentation or test cases, it's hard to criticize. What's important is that your implementations are doing what your documentation says. Also, it's very important to create a test suite for such modules and perhaps a performance test suite as well.

\$\endgroup\$
4
  • \$\begingroup\$ Very elegant. I see that I should be more specific about code's intensions. For 'isplainobj ' I ment to have the behaviour you pointed out ( wheather or not in inheritance tree ) to detect custom made types. In my experience they behave exactly like plain ( Object ) types. If 'for in' starts iterating something ( inherited or no ) then there is some info to deal with, thats isemptyobj's implementation goal; \$\endgroup\$ Sep 30, 2013 at 23:28
  • \$\begingroup\$ 'isnumeric's' implementation is built over a period of time ( that explains weird syntax ), and I know there is one test case ( which I sadly can not remember now of why I've used it like that ) for which I've used '["valueOf"]()' thing, where response should be positive... Btw your 'isnumeric' validates as positive for blank strings of any length and booleans, because .isFinite() is doing type conversion when doing it's thing. Smart use of object lookups and typeof operator. 'void 0' is redundat, I've used it to just to circumvent code warning underlines code editor was giving me. \$\endgroup\$ Sep 30, 2013 at 23:53
  • 1
    \$\begingroup\$ @NikolaVukovic You are right, I forgot about these cases. I updated my answer accordingly and added another idea. A generic is function that checks if a value's type matches one of the provided types. \$\endgroup\$
    – plalx
    Oct 1, 2013 at 2:00
  • \$\begingroup\$ sounds resonable, I'll post the possible solution once implemented/tested, etc. \$\endgroup\$ Oct 1, 2013 at 2:10
1
\$\begingroup\$

Did you run this through a minifier and then prettified it again?

Stuff like (foo) && (bar || (x = y())) looks like google closure compiler and has no place in actual source code. It makes your code hard to read and letting closure compiler do this later is perfectly fine - no need to work with less readable code.

\$\endgroup\$
4
  • \$\begingroup\$ ahm, no, I tend to use logical operators instead of conditional statements often. \$\endgroup\$ Sep 28, 2013 at 18:17
  • \$\begingroup\$ Please don't. Especially when you start putting assignments inside. \$\endgroup\$ Sep 28, 2013 at 18:23
  • \$\begingroup\$ It's a kind of a ( bad? ) habit trying to speed up typeing... \$\endgroup\$ Sep 28, 2013 at 18:25
  • \$\begingroup\$ @ThiefMaster I believe that simple expressions, such as callback && callback() are perfectly fine and readable. It's an elegant way of taking advantage of the short-circuit nature of the && operator. \$\endgroup\$
    – plalx
    Oct 1, 2013 at 12:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.