JavaScript code design

Question

The question it more about the design of the code and how it could be written better. The code itself works as it should.

A little about the code itself: the function checks the object passed as the first argument and either return a string with the type of object (array, date, xmlhttprequest, etc.) or a Boolean when comparing an object with a string value.

The function can also iterate through an array of objects and returns a Boolean value, an array of Boolean results, or an object.

(function ( global ) {
  "use strict";
  Object.type = function type( testObject, testAgainst, returnType ) {
    var result, getType = function ( object ) {
      var result;
      if ( object && object.nodeType !== undefined ) {
        result = 'dom';
      }
      else if ( object === global ) {
        result = 'global';
      }
      else {
        result = ({}).toString.call( object ).match( /\s([a-zA-Z]+)/ )[1].toLowerCase();
      }
      return result;
    };

    if ( getType( testAgainst ) !== 'undefined' ) {
      if ( getType( testAgainst ) === 'string' ) {
        return getType( testObject ) === testAgainst;
      }
      else if ( getType( testAgainst ) === 'array' ) {
        if ( getType( returnType ) === 'undefined' ) {
          returnType = 'boolean';
        }
        result = {
          'boolean': function () {
            return testObject.every(function ( member, index ) {
              return getType( member ) === testAgainst[index];
            });
          },
          'array': function () {
            return testObject.map(function ( member, index ) {
              return getType( member ) === testAgainst[index];
            });
          },
          'object': function () {
            var result = {};
            testObject.forEach(function ( member, index ) {
              result[ getType( member ) ] = getType( member) === testAgainst[index];
            });
            return result;
          }
        };
        return result[ returnType ]();
      }
    }
    return getType( testObject );
  };
}( this ));

Usage examples:

  (function () {
    var objects = [null, 0, undefined, new Date(), new XMLHttpRequest(), function () {}, {}, [], 'string'],
      matches = ['null', 'number', 'undefined', 'date', 'xmlhttprequest', 'function', 'object', 'array', 'string'],
      misses = ['array', 'number', 'string', 'date', 'xmlhttprequest', 'function', 'object', 'number', 'string'];

    console.dir({
      "objects array: ": objects,
      "matches array: ": matches,
      "misses array: ": misses,
      "Object.type( {} )": Object.type( {} ), //returns 'object'
      "Object.type( 2013, 'number' )": Object.type( 2013, 'number' ), //returns true
      "Object.type( objects, matches )": Object.type( objects, matches), //returns true
      "Object.type( objects, misses )": Object.type( objects, misses ), //returns false
      "Object.type( objects, matches, 'object' )": Object.type( objects, matches, 'object' ),
      "Object.type( objects, matches, 'array' )": Object.type( objects, matches, 'array' ), //returns Array[9] (true, true, true, true, true, true, true, true, true)
      "Object.type( objects, misses, 'object' )": Object.type( objects, misses, 'object' ),
      "Object.type( objects, misses, 'array' )": Object.type( objects, misses, 'array' ) //returns Array[9] (false, true, false, true, true, true, true, false, true)
    });
  }());

The problem with the design that mainly bugs me is that the majority of the code is inside nested if statements.

What design changes I could make to this code to make it ready for production?

Edit:
I've created a gist where I updated the above code based on comments (https://gist.github.com/silverstrike/5108601)

@Oliver: Nothing wrong with `if` conditions. But It seem to me that if the majority of the code is inside nested conditions, then the design could be improved. — silverstrike, Mar 07 '13 at 13:32
@silverstrike: If conditions are perfectly fine. The only situations where you shouldreally avoid them is if you want to be able to [add new "cases" in the future](http://c2.com/cgi/wiki?ExpressionProblem). — hugomg, Mar 07 '13 at 13:41
No, this is definitely not "fine". Even for JS where you sometimes have to do some pretty ugly stuff, this is really illegible code. There's quite a difference between there-I-hacked-it-and-it-works and "fine". — Creynders, Mar 07 '13 at 13:56
@minitech: Thanks. I had no idea StackExchange had such a site :). Will post there as well — silverstrike, Mar 08 '13 at 00:38

score 0 · Accepted Answer · edited May 23 '17 at 12:32

0

You should use early returns (there's no good reason not to use them), it will simplify your code a lot and un-nest all those if-statements. For example, getType becomes this:

function getType(object) {
    if (object && object.nodeType !== undefined)
        return 'dom';
    if (object === global)
        return 'global';
    return ({}).toString.call(object).match(/\s([a-zA-Z]+)/)[1].toLowerCase();
}

You are using the other Alternative to a million IF statements (the object literal) already, yet you might choose only one of them instead of mixing them. Especially in your case, where you have an object of functions to be executed, some if-statements would be much shorter/easier to read/write.

edited May 23 '17 at 12:32

Community

1
1

answered Mar 07 '13 at 13:34

Bergi

630,263
148
957
1,375

I initially used that approach, but JSlint.com had issues with it. Is that an issue with jslint? – silverstrike Mar 07 '13 at 13:39
What issues? JSlint can only give you hints, and if they are leading to illigible code they should not be obeyed. – Bergi Mar 07 '13 at 13:42
I usually try to understand those hints (and usually I come to agree with them). I am currently reading through the topic you linked to, and I will change it back. – silverstrike Mar 07 '13 at 13:51
Based on your suggestions I've created a gist (https://gist.github.com/silverstrike/5108601) where I updated the code. Would appreciate more comments :). Thank you. – silverstrike Mar 07 '13 at 15:16

JavaScript code design

1 Answers1