Bulletproof way to clone Objects & Arrays handling circular references?

Just want to know if this is safe way to clone arrays and plain objects avoiding infinite loops trigger by circular refrences:

```
//
!(( function ( methodName, dfn ) {
// add global 'clone' method identifier
this[methodName] = dfn();
} ).call(
self,
"clone",
function () {

var _api = {

// two arrays to keep track of what was cloned
originalValues : [],
clonedValues : [],

clone : function ( input ) {
throw new Error(
"Failed to clone value: " + input + "."
);
},

ownEach : function ( obj, func ) {
for ( var p in obj ) {
obj.hasOwnProperty( p )
&& func.call( obj, p, obj[p] );
}
return obj;
}
};

// helper functions
function corstr( o ) {
return Object.prototype.toString.call( o );
}
function corslice( args, i1, i2 ) {
return Array.prototype.slice.call( args, i1, i2 );
}
function isArray( o ) {
return corstr( o ) === "[object Array]";
}
function isObject( o ) {
return corstr( o ) === "[object Object]";
}
function bind( obj, fn ) {
return function () {
return fn.apply( obj, corslice( arguments ) );
};
}
function override( obj, method_v1, method_v2 ) {
var coreMethod = obj[method_v1];
return obj[method_v1] = function () {
return method_v2.apply(
this,
[ bind( this, coreMethod ) ]
.concat( corslice( arguments ) )
);
};
}

//
// build up clone method

// handles arrays and objects
override(
_api,
"clone",
function ( oldMethod, input ) {
var cloned, isarray = false;
if (
isArray( input )
&& ( isarray = true, cloned = [] )
|| isObject( input )
&& ( cloned = {

What the tin says

Your code seems to track all cloned objects, so that it will find circular references, so it should be bullet proof indeed.

The goggles!

!(( function ( methodName, dfn ) {
    // add global 'clone' method identifier
    this[methodName] = dfn();
} ).call(
self,
"clone",
function () {

• There seems to be no good reason to negate the outcome of that function

• Splitting function parameters over different lines should be avoided unless it makes your code more readable, that is definitely not the case here! You do this several times in the code.

Also this:

_api.originalValues.length
    = _api.clonedValues.length
    = 0;

looks so much better as

_api.originalValues.length = _api.clonedValues.length = 0;

finally, stuff like this:

&& (
              input
              .forEach(
                function ( val ) {
                    cloned.push( _api.clone( val ) );
                }
              ),
              true
            )

Might have some artistic value, but it makes your code far too hard to grok.

Overkill

clone   : function ( input ) {
  throw new Error(
    "Failed to clone value: " + input + "."
  );
},

I assume you are very proficient in another OO language, but things like creating a useless clone function have absolutely no value in JavaScript, but it can eat up bandwidth and cpu time.

Naming

• You are not consistent with lowerCamelCase

• I understand why you call these corstr and corslice, they are still bad names

• Having a function isArray and a var isarray is mildly evil

Checking on Arrays and Objects

I really like this implementation:

function isType(object, type){
     return object != null && type ? object.constructor.name === type.name : object === type;
}

Then you can

if( isType( input , Array ) )

Scope

return function ( value ) {
    var out = _api.clone( value );
    // empties arrays after _api.clone() func is done
    _api.originalValues.length
    = _api.clonedValues.length
    = 0;
    return out;
};

is wrong, why should the caller of clone need to worry about originalValues and clonedValues ? These should be part of clone so that no house keeping is required. Then you can simple have

return _api.clone;

Advanced Hackery

• Assignments inside if statements are cool, but should only be used in personal projects. They are a source of bugs and confusion for other developers.

• Writing you own bind is great, but I would suggest you check out the bind shim of Mozilla.

Finale

I would not want to work with this code until it got beautified and all jshint trouble fixed. It works, but I am certain that a far shorter and far more readable implementation can be written.

StackExchange Code Review Q#31898, answer score: 5