0

I am trying to modify a single element from an array whose elements were previously duplicated n times. To perform the array duplication I just relied on a custom function duplicateElements(array, times)from this post (see @Bamieh answer). As shown in the exemple below, the problem is I can't modify a single element from the array without modifying other elements:

function duplicateElements(array, times) {
  return array.reduce((res, current) => {
    return res.concat(Array(times).fill(current));
  }, []);
}

var myvar = duplicateElements([{ a: 1 }, { a: 2 }], 2);
myvar[0].a = 3;

console.log(myvar);
// (4) [{…}, {…}, {…}, {…}]
// 0: {a: 3}
// 1: {a: 3}
// 2: {a: 2}
// 3: {a: 2}
// length: 4

As you can see myvar[1].a was also modified although this wasn't intended. How can I avoid this issue?

mat
  • 2,412
  • 5
  • 31
  • 69
  • If your objects can have multiple nested levels, you will have to search about deep cloning of an object, so you can do something like: `res.concat(Array(times).fill(clone(current)));`. Otherwise you can use [spread syntax](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax) like: `res.concat(Array(times).fill({...current}))` – Shidersz May 27 '19 at 17:50
  • @Shidersz I tried `_.cloneDeep(current)` from lodash, but it didn't work. – mat May 27 '19 at 18:25
  • How it didn't work, update with the code you have tried, I just read about that method from `Lodash`, just by curiosity, and it worked fine on some tests I made. If you want to use `Lodash` I could show you an example. – Shidersz May 27 '19 at 19:19

3 Answers3

1

The problem is that you're passing the reference to the original object in Array(times).fill(current) .

In this case the two copies of the first {a:2} are the same copy of the original (They reference to the same space in memory) so if you change one, the two of them will change as they reference the same object in memory.

You have to make a deepcloning function or maybe spread the object inside a new one. You can change your original function to work with objects and primitives like this:

function duplicateElements(elementsArray, times) {
  //Make a new placeholder array
  var newArray = [];
  //Loop the array of elements you want to duplicate
  for (let index = 0; index < elementsArray.length; index++) {
    //Current element of the array of element
    var currentElement = elementsArray[index];
    //Current type of the element to check if it is an object or not
    var currentType = typeof currentElement
    //Loop over the times you want to copy the element
    for (let index = 0; index < times; index++) {
      //If the new element is not an object
      if (currentType !== "object" && currentType){
        //append the element
        newArray.push(currentElement)
        //if it is an Object
      } else if (currentType === "object" && currentType){
        //append an spreaded new Object https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax
        newArray.push({...currentElement})
      }
    }
  }
  return newArray;
}

This is not the optimal way to do this, but I think that maybe you're new to javascript and is better to learn the old way of looping before using more Array functionalities (as the answer from Jonas Wilms, that is also a good answer).
I would recommend javascript.info and eloquent javascript to learn more about the language

1

The main reason for this as specified in the Array.fill documentation is that when dealing with objects it will copy by reference:

When fill gets passed an object, it will copy the reference and fill the array with references to that object.

With lodash (and _.cloneDeep) that is one line like this:

let dubFn = (arr, t=1) => 
  _.concat(arr, _.flatMap(_.times(t, 0), x => _.cloneDeep(arr)))

let r1 = dubFn([{a:1},{b:3}])            // no parameter would mean just 1 dub
let r2 = dubFn([{a:1},{b:3},5,[1]], 2)  // 2 dublicates

r1[0].a = 3
r2[0].a = 3

console.log(r1)
console.log(r2)
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.11/lodash.min.js"></script>

Note that this now works with arrays/objects and primitives.

The idea is to use _.concat to return a new concatenated version of the input array with a combination of few functions which on the end return an array of cloned objects. We use _.times to return an array of in this case t elements and then for each of those elements we replace with a deep clone of the array. _.flatMap is needed to flatten the end result since we end up having array of arrays after the _.times call.

With ES6 you can do something like this:

let dubElements = (arr, t) => 
  [...arr, ...new Array(t).fill().flatMap(x => arr.map(y => ({...y})))]

let r1 = dubElements([{a:1},{b:3}])
let r2 = dubElements([{a:1},{b:3}],2)

r1[0].a = 3
r2[0].a = 3

console.log(r1)
console.log(r2)

Where we concat arrays via the spread operator and we use new Array(t) to create the new duplicates array and make sure we fill it with undefined in this case after which we flatMap the results (which we map through the clone via the spread operator again.

Note that this works for your use case specifically. If you want to make it more generic you have to expand more in the last map function etc.

If you want to preserve the order of the elements you can do something like this:

let dubElements = (arr, t=1) => {
  let _result = [] 
  arr.forEach(x => {
    for(let i=0; i<t+1; i++) {
      _result.push({...x})
    }
  })
  return _result
}

let result = dubElements([{a:1},{b:3}],2)

result[0].a = 3

console.log(result)
Akrion
  • 18,117
  • 1
  • 34
  • 54
  • Is there a way to conserve the order of elements (e.g., ["a", "a", "b", "b"], instead of ["a", "b", "a", "b"]) – mat May 28 '19 at 13:22
  • 1
    Updated the answer with a method where the order is conserved. – Akrion May 28 '19 at 17:40
0

Replace

 Array(times).fill(current)

which will add one reference to current multiple times to the array with:

  Array.from({ length: times }, () => ({...current }))

which will shallow clone current. Note that the code will then only work with objects though, not with primitives.


I'd do:

 const duplicateElements = (array, length)  => 
    array.flatMap(current => Array.from({ length }, () => ({ ...current }));
Ori Drori
  • 183,571
  • 29
  • 224
  • 209
Jonas Wilms
  • 132,000
  • 20
  • 149
  • 151
  • Your custom function (`const...`) works perfectly with my exemple but not with `duplicateElements(["aa", "bb"], 2)`, where the expected result should be `["aa", "aa", "bb", "bb"]`. Is there a way to make it work in both cases? – mat May 27 '19 at 18:10
  • @mat you need some way to deep clone, you'll find some implementations if you search for it. – Jonas Wilms May 28 '19 at 07:41