2
\$\begingroup\$

I'm a beginner to JavaScript and today I learned about primitive and reference values, so I decided to write a function that can deep clone an array and objects. Is this good code or is it not good enough?

function clone(o,mode){
  'use strict';
  mode = mode || 'deep';
  var obj = {} ;
  var arr = [];
  if(typeof o === "object"){
    if(Array.isArray(o)){
      //is Array
      if(mode!=="deep"){
        arr =  o.slice();
      }else{
      for (var i = 0, len = o.length; i < len; i++) {
        if(typeof o[i] === "object") arr.push(clone(o[i],1));
        else arr.push(o[i]);
      }}
      return arr;

    }else{
      //is Object
      if(mode!=="deep"){
        obj =  Object.assign({},o);
      }else{
      for (var key in o) {
    // skip loop if the property is from prototype
    if (!o.hasOwnProperty(key)) continue;
        if(typeof o[key] === "object")obj[key]=clone(o[key],1);
        else obj[key]=o[key];
      }}
      return obj;

  }

}
    return o;
}
\$\endgroup\$
3
\$\begingroup\$

What I would normally suggest is to NOT do this by yourself. There's too many data types in JavaScript and they all have their quirks when it comes to deep cloning. For instance, what would your code do if it encountered a function? A regular expression? A date object? Map? Set? etc. There's also cyclic references.

Many libraries have this functionality built-in. I usually recommend Lodash cloneDeep. If you happen to use jQuery, it's $.extend more or less does the same thing. If you're curious how they're implemented, they're both open-source and on Github.

Now if you reaaally just want to try to do this yourself:

mode = mode || 'deep';

ES6+ now has default parameters.

for (var i = 0, len = o.length; i < len; i++) {
  if(typeof o[i] === "object") arr.push(clone(o[i],1));
  else arr.push(o[i]);
}}

You can replace this portion with array.map.

if (!o.hasOwnProperty(key)) continue;
  if(typeof o[key] === "object")obj[key]=clone(o[key],1);
  else obj[key]=o[key];
}}

You can use Object.keys to get enumerable keys and array.reduce to generate the object.

\$\endgroup\$
  • 1
    \$\begingroup\$ @JeremyBanks But then 1. _.cloneDeep already does that and 2. The OP's code doesn't do that. So the argument is moot. \$\endgroup\$ – Joseph Apr 11 '17 at 17:22
3
\$\begingroup\$

I would agree with answer from @JosephtheDreamer that, as a beginner in javascript, this is probably a domain area you don't want to tackle unless your intent is just to deep clone simple array and object literal type data structures.

A few thoughts on what you have beyond what Joseph pointed out:

  • Should your mode parameter be boolean and possibly named deepClone, recursive or similar? You only have two "modes" here, so using a string value for this toggle seems strange. Oddly enough, your recursive calls to clone() pass 1 as value for "mode" so your code actually has a bug in it right now, as all your recursive calls would not trigger mode conditionals correctly.
  • Why nest all that code inside (typeof o === "object") conditional? Consider inverting this conditional and returning early in this case, allowing to de-nest most of the logic in this function.
  • Similarly, you have no need for an else condition at all for the non-array case, as the array case will always return. You should always look for ways to remove unnecessary code paths in your code. The more code paths you have, the harder code is to test/maintain, and the more fragile your code becomes.
  • You could also get rid of your main if-else conditionals related to "mode" in each of your array/non-array branches by returning early.
  • Stylistically, you could clean this up a lot to make it easier to read. Indentation is poor. Variable names should be more meaningful and not truncated for shortness' sake. Use human readable variables and let a minifier do the work on shortening your code for deployment. Spacing around assignment operators, comparison operators, flow control constructs, and function calls is inconsistent. Brackets for conditionals are used inconsistently (especially looking odd with you most deeply nested if-else conditions).

I would also strongly suggest that, if you are just learning javascript, you do so based on ES6 and get familiar with that syntax. If you need to deploy to browsers that do not fully support ES6 (this would probably be the case for most general web applications), than use a transpiler like Babel to transpile your code for older ECMAscript versions.

\$\endgroup\$
1
\$\begingroup\$

Your code fails to clone beyond depth 2. I mean when you do as follows;

var obj = {a: 1, b: {c: 2, d: {e: 3, f: {g: 4, h: null}}}},
  clobj = clone(obj);
clobj.b.d.e = "three";
concole.log(obj.b.d.e, clobj.b.d.e) // <- "three" "three"

which means clobj still has reference objects to the corresponding ones in the obj.

Accordingly considering the main object type elements of JS, such as objects, arrays and null one might do deep cloning as follows;

function objectClone(o){
  var ot = Array.isArray(o);
  return o !== null && typeof o === "object" ? Object.keys(o)
                                                     .reduce((r,k) => o[k] !== null && typeof o[k] === "object" ? (r[k] = objectClone(o[k]),r)
                                                                                                                : (r[k] = o[k],r), ot ? [] : {})
                                             : o;
}

var obj = {a: 1, b: {c: 2, d: {e: 3, f: {g: 4, h: null}}}},
    arr = [1,2,[3,4,[5,6,[7]]]],
    nil = null,
  clobj = objectClone(obj),
  clarr = objectClone(arr),
  clnil = objectClone(nil);
console.log(clobj, obj === clobj);
console.log(clarr, arr === clarr);
console.log(clnil, nil === clnil);
clarr[2][2][2] = "seven";
console.log(arr, clarr);
.as-console-wrapper { max-height: 100% !important; top: 0; }

Yes, in JS there are tons of objects but among them functions have a special place since they can be constructors and typeof check will return "function". So according to the code above they can not be cloned.

\$\endgroup\$

Your Answer

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

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