# Criticize my JavaScript value tester suite

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; } }; // } ));  ## 2 Answers 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


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.

• 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; – Nikola Vukovic Sep 30 '13 at 23:28
• '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. – Nikola Vukovic Sep 30 '13 at 23:53
• @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. – plalx Oct 1 '13 at 2:00
• sounds resonable, I'll post the possible solution once implemented/tested, etc. – Nikola Vukovic Oct 1 '13 at 2:10

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.

• ahm, no, I tend to use logical operators instead of conditional statements often. – Nikola Vukovic Sep 28 '13 at 18:17
• Please don't. Especially when you start putting assignments inside. – ThiefMaster Sep 28 '13 at 18:23
• It's a kind of a ( bad? ) habit trying to speed up typeing... – Nikola Vukovic Sep 28 '13 at 18:25
• @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. – plalx Oct 1 '13 at 12:03