2
\$\begingroup\$

I've made this function for to get a deep copy (recursive copy) of objects. => Sub-objects aren't references to the sub-objects of the original object. Instead they are objects on their own.

Here's my code ...

Attention: Most of it are test data and an additional test-function.

// ----- Test data ---------------------		
var swapi = [ {
  "name": "Luke Skywalker",
  "height": "1.72 m",
  "mass": "77 Kg",
  "hair_color": "Blond",
  "skin_color": "Caucasian",
  "eye_color": "Blue",
  "birth_year": "19 BBY",
  "gender": "Male",
  "homeworld": "http://swapi.co/api/planets/1/",
  "films": [
    "http://swapi.co/api/films/1/",
    "http://swapi.co/api/films/2/",
    "http://swapi.co/api/films/3/"
  ],
  "species": [
    "http://swapi.co/api/species/1/"
  ],
  "vehicles": [
    "http://swapi.co/api/vehicles/14/",
    "http://swapi.co/api/vehicles/30/"
  ],
  "starships": [
    "http://swapi.co/api/starships/12/",
    "http://swapi.co/api/starships/22/"
  ],
  "created": "2014-12-09T13:50:51.644000Z",
  "edited": "2014-12-10T13:52:43.172000Z",
  "url": "http://swapi.co/api/people/1/"
},
             {
             "name": "C-3PO",
             "height": "167",
             "mass": "75",
             "hair_color": "n/a",
             "skin_color": "gold",
             "eye_color": "yellow",
             "birth_year": "112BBY",
             "gender": "n/a",
             "homeworld": "http://swapi.co/api/planets/1/",
             "films": [
             "http://swapi.co/api/films/5/",
             "http://swapi.co/api/films/4/",
             "http://swapi.co/api/films/6/",
             "http://swapi.co/api/films/3/",
             "http://swapi.co/api/films/2/",
             "http://swapi.co/api/films/1/"
            ],
    "species": [
      "http://swapi.co/api/species/2/"
    ],
      "vehicles": [],
        "starships": [],
          "created": "2014-12-10T15:10:51.357000Z",
            "edited": "2014-12-20T21:17:50.309000Z",
              "url": "http://swapi.co/api/people/2/"
},
  {
    "name": "R2-D2",
      "height": "96",
        "mass": "32",
          "hair_color": "n/a",
            "skin_color": "white, blue",
              "eye_color": "red",
                "birth_year": "33BBY",
                  "gender": "n/a",
                    "homeworld": "http://swapi.co/api/planets/8/",
                      "films": [
                        "http://swapi.co/api/films/5/",
                        "http://swapi.co/api/films/4/",
                        "http://swapi.co/api/films/6/",
                        "http://swapi.co/api/films/3/",
                        "http://swapi.co/api/films/2/",
                        "http://swapi.co/api/films/1/",
                        "http://swapi.co/api/films/7/"
                      ],
                        "species": [
                          "http://swapi.co/api/species/2/"
                        ],
                          "vehicles": [],
                            "starships": [],
                              "created": "2014-12-10T15:11:50.376000Z",
                                "edited": "2014-12-20T21:17:50.311000Z",
                                  "url": "http://swapi.co/api/people/3/"
  }
];

// --- Start : THE ACTUAL FUNCTION ---------------

/** Supplies a deep copy of an object.
 * 
 * @param { Object } obj - The object to copy.
 * @return { Object } - Copy of the given object.
 * @throws { Object } When given object is not 
 *   of type object.
 *
 * -- Usage example -------------------------------
 * var copyOfObj = getDeepCopy({ a: 3, b: 5 });
 */

function getDeepCopy(obj) {
  var ret;

  if (typeof obj !== 'object') {
    throw {
      message: 'Object expected as parameter.'
    }
  }

  ret = Array.isArray(obj) ? [] : {};

  for (var key in obj) {
    if (obj.hasOwnProperty(key)) {		  
      let current = obj[key];

      if (typeof current !== 'object') {
        ret[key] = current;
      } else {		
        ret[key] = getDeepCopy(current);
      }
    }
  }

  return ret;
}
// --- End : THE ACTUAL FUNCTION ------------------

// -------- TESTING --------------------------------

function testGetDeepCopy(original, copy) {
  var originalLength = Object.keys(original).length;
  var copyLength = Object.keys(copy).length;
  var errorMessages = '';	  		  

  if (originalLength !== copyLength) {  
    errorMessages += 'Count properties original is ' +
      originalLength + ' but count of properties copy is ' +
      copyLength + '.'
  }

  for (var prop in original) {
    if (typeof original[prop] !== 'object') {
      if (original[prop] !== copy[prop]) {
        errorMessages += ' Original value is ' + original[prop] +
          ' but copy value is ' + copy[prop] + '.';
      }
    } else {				
      if (original[prop] === copy[prop]) {
        errorMessages +=
          ' Original and copy reference is same object.';
      } else {
        let tmp = testGetDeepCopy(original[prop], copy[prop]);

        if (tmp !== 'Test passed.') {
          errorMessages + ' ' + tmp;
        }
      }
    }
  }

  if (!errorMessages) {
    return 'Test passed.';
  } else {
    return errorMessages.trim();
  }
}

try {
  swapi.forEach(function(swObj, i) {
    console.log('Test %s (%s) : %s',
                i,
                swObj.name,
                testGetDeepCopy(swObj, getDeepCopy(swObj)));
  });
} catch (e) {
  console.log(e.message);
}

I leave it in there because I would be interested in:

  • Hints and comments to the actual function: Would it work as expected? Could it be improved. Or would be better to take a complete different approach?

  • Documentation: Done a proper way? Can one understand at once how the function has to be used? Or must it be improved?

  • Testing: Does it cover everything what is important? Possible improvements?

Looking forward to read your answers.

\$\endgroup\$
  • \$\begingroup\$ Chromium/WebKit js shim layer (and probably lots of other js software) simply uses var deepCopy = JSON.parse(JSON.stringify(source)) for the simple data shown in your code. Do some benchmarks to compare the speed of your solution. \$\endgroup\$ – wOxxOm Sep 16 '16 at 10:30
  • \$\begingroup\$ Great tipp one really has to know. :) Thanks. \$\endgroup\$ – michael.zech Sep 17 '16 at 7:42
3
\$\begingroup\$

Here some points I got from your code.

The use of var and let.

The var keyword define a variable with a scope in the function where the variable is declared.

The let keyword define a variable with a scope local.

In your case the ret variable could be also declared using the let keyword, and it works as it is declared in the top of the function, so the local scope is the whole function.

But, in this case you're right to use var, just to determined that the variable have a function scope.

Instead you should use let in the for loop:

for (var key in obj) {
    ...

Here the variable key should not have a function scope at all.

The second point is still in your for loop.

You check the key if exists inside the object obj, but you don't need this check, as you are using the in operator that get an object real key.

So just remove this part.

I don't know where you use this code, so I'm not sure about the useful of this suggestion, you could add a parameter to limit the deep level you're copying.

The reason is to avoid security issues in case of very deep object structures, and circular references.

You could check if the parameter is set and limit the copy just in this cases.

About improving your test.

To be sure that the copy is a different object, and not just a reference, you should add some negative checks, like changing the original object and test that the value in the copy object is not changed, or add a new property, and check that is not in the copy.

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

Be careful with the typeof operator, because in javascript null is a Object.

typeof null === "object" // this returns true

You need to make sure that the function will not have a strange behavior when null is passed to it.

The array being tested only have strings and arrays as children, you should try adding other types of properties like undefined, integers or function (if necessary).


you should take a look in the underscore library, its a excellent 'utility-belt library', and even if you dont wanna use it in your code, you can use it as a reference. (though, they don't have a deepCopy function)

\$\endgroup\$
  • \$\begingroup\$ You are right. Not checking for null was definitely a weak point in there. Thanks a lot. \$\endgroup\$ – michael.zech Sep 17 '16 at 7:41

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.