Adding arrays to an array of arrays

Question

I'm sure this is super simple, and I don't really want a complete solution, but pointing in the right direction as I'm learning.

I have:

let randomArray = [1,2,4,591,392,391,2,5,10,2,1,1,1,20,20];

and the goal is to make it look like this, grouping like items:

[[1,1,1,1],[2,2,2],[10],[20,20],[391],[392],[591]]

My following code arranges, and groups fine. I push the temp to my Group array. But when I reset my "tempArray" to make it empty ready for the next "group", it removes the data from my group as well. Because it's linked? I assume? Maybe?

The only thing left at the end is the last item.

How do I stop it doing this?

    // My SOlution
let randomArray = [1,2,4,591,392,391,2,5,10,2,1,1,1,20,20];
let tempArray = [];
let groupArray = [];

function cleanTheRoom (arr) {
    let ascendingArray = arr.sort(function(a,b) {
        return a - b;
    });
    tempArray.push(randomArray[0])
    for (let i = 1; i <= randomArray.length; i++) {
        if (randomArray[i] === tempArray[0]) {
            tempArray.push(randomArray[i])
        } else {
            groupArray.push(tempArray);
            tempArray = []
            tempArray.push(randomArray[i])
        }
    } console.log(groupArray)
}

cleanTheRoom(randomArray);

Ungodly

The issue is just here: tempArray.length = 0 which should become tempArray = []. Simple as that. You should assign a new array to tempArray, otherwise all the tempArray references will need to keep a reference to the original values, which is why you get the strange logs. There are faster (and easier) ways to do that, though. – Modena

So close yet so far. Thank you so much, all I did was change that to: tempArray = [] And it works! Really appreciate that. – Ungodly

I've seen a solution that is faster, but I wanted to work one out myself to learn more from the process. But got stuck here. More of a practice exercise. – Ungodly

To be entirely correct, I think you could also: groupArray.push([...tempArray]). That would do the job as well as long as your array only holds primitives. This does the job because it pushes the array primitive values to the groupArray, which won't be linked to the tempArray anymore. Either way, it should work as intended. – Modena

Also make sure that you push the temp array in the groupArray array in the last iteration of the loop otherwise the groupArray won't contain the last number, i.e. 591 – Nador

groupArray.push([...tempArray]) doesn't appear to work in the same way, it just lists them. I may be doing something wrong though. I have noticed that the last item did not push, and I will do exactly that, thank you. – Ungodly

Answer 1

I took your code and changed some parts.

• Array#sort sorts in place. No need to assign to a new variable.

• Iterate from zero and have a look to the data directly, by using the value at index minus one and at the index. If unequal, a new group is found. In this case just assign an empty array to groupArray and push the group to the result.

  Instead of using the same object reference and just empty the array by assigning zero to length, this approach takes a new array.

• Push the value outside of the if statement, because it was doubled in then and else part.

• Finally return the array with the groups.

function cleanTheRoom(array) {
    let result = [];
    let groupArray;

    array.sort(function(a, b) {
        return a - b;
    });

    for (let i = 0; i < array.length; i++) {
        if (array[i - 1] !== array[i]) {
            groupArray = [];
            result.push(groupArray);
        }
        groupArray.push(array[i]);
    }

    return result;
}

let randomArray = [1, 2, 4, 591, 392, 391, 2, 5, 10, 2, 1, 1, 1, 20, 20];

console.log(cleanTheRoom(randomArray));

Liva

Thanks, really clean compared to mine, and you've explained a few things that will help going forward. So, array[i-1] has to be false from initial inception, and thus immediately an array is created? I've had to run it through debugger to understand it as it's the opposite to how my brain appears to work (if equal, skip the if statement and add it to the groupArray, otherwise create a new array). – Ungodly

right. sometimes an inverted view is shorter than the obvious. – Liva

Answer 2

There are three main problems in your code:

1. You are using randomArray inside the function but you should be using ascendingArray

2. tempArray.length = 0 - this statement mutates the original tempArray and since you are pushing the tempArray array in the groupArray, changes to tempArray are reflected in the groupArray as well.

   You could clone the tempArray and push the copy of the tempArray in the groupArray

   groupArray.push([...tempArray]);
   

   or you could assign the new empty array to tempArray

   tempArray = [];
   

3. When its the last iteration of the loop, you are not pushing the contents of the tempArray in the groupArray. This will lead to the groupArray not containing the last number in the sorted array, i.e. 591. You need to check if the current iteration is the last iteration of the loop or not. If it is, push the tempArray in the groupArray

   for (let i = 1; i < ascendingArray.length; i++) {
     ...
   
     if (i == ascendingArray.length - 1) {
       groupArray.push(tempArray);
     } 
   } 
   

Here's a simplified version of your code:

let randomArray = [1, 2, 4, 591, 392, 391, 2, 5, 10, 2, 1, 1, 1, 20, 20];

function cleanTheRoom(arr) {
  let tempArray = [];
  let groupArray = [];

  let ascendingArray = arr.sort(function (a, b) {
    return a - b;
  });

  for (let i = 0; i < ascendingArray.length; i++) {
    tempArray.push(ascendingArray[i]);

    if (ascendingArray[i + 1] !== ascendingArray[i]) {
      groupArray.push(tempArray);
      tempArray = [];
    }
  }

  console.log(groupArray);
}

cleanTheRoom(randomArray);

Nador

Assigning tempArray = [ ] appeared to work well, but groupArray.push([...tempArray]); doesn't seem to do what I require. I managed to get the last item by changing my range to i <= ascendingArray.length but is this bad practice? – Ungodly

Can you edit your question and show the updated code where you used groupArray.push([...tempArray]);? It should work with only one change in your code. – Nador

I have seen the updated code, problem is that you are not resetting the length of tempArray or setting tempArray to a empty array. – Nador

I also commented out the tempArray = [] as I assumed it was to replace this? Otherwise, just using tempArray = [] without [...tempArray] change from tempArray works – Ungodly

My new edit works fine with just the = [] change. What benefits does [...tempArray] have? – Ungodly

[...tempArray] creates a copy of the tempArray so even if you do tempArray.length = 0, it won't affect the array pushed in the groupArray because groupArray will contain the copy of the tempArray – Nador

Right, I see! That's really helpful, thank you. So it clones it, and I could use tempArray.length = 0; though probably no ideal; as it is effectively a new array – Ungodly

Yes, tempArray.length = 0 won't affect the copied array. – Nador

Answer 3

There might be even faster approaches but I took a quick shot at it:

• first, we break down the array into the groups
• and then we sort not on the whole array but only on the group's key to avoid a second full iteration over the array.

I've used a Map instead of a dictionary for the groups because we can leverage the Map.set function neatly. It returns the whole Map which we need as the return value within the reduce.

const randomArray = [1, 2, 4, 591, 392, 391, 2, 5, 10, 2, 1, 1, 1, 20, 20];
const isGroup = (acc, number) => Array.isArray(acc.get(number));
const setGroup = (acc, number) => acc.set(number, isGroup(acc, number) ? acc.get(number).concat([number]) : [number]);
const unorderedNumberGroups = randomArray.reduce(setGroup, new Map());
const order = [...unorderedNumberGroups.keys()].sort((a, b) => a - b);
const orderedNumberGroups = order.reduce((acc, key) => acc.concat([unorderedNumberGroups.get(key)]), []);
console.log(orderedNumberGroups);

Here you go with a more debuggable version so you can try to understand the version above:

const randomArray = [1, 2, 4, 591, 392, 391, 2, 5, 10, 2, 1, 1, 1, 20, 20];
const isGroup = (acc, number) => Array.isArray(acc.get(number));
const unorderedNumberGroups = randomArray.reduce((acc, number) => {
  if (isGroup(acc, number)) {
    const mapEntry = acc.get(number);
    const newEntry = mapEntry.concat([number]);
    return acc.set(number, newEntry); // we need to return the acc, the Map.set method returns the whole Map
  } else {
    return acc.set(number, [number]); // we need to return the acc, the Map.set method returns the whole Map
  }
}, new Map()); // initialize the accumulator we an empty Map
const keysAsArray = [...unorderedNumberGroups.keys()];
const order = keysAsArray.sort((a, b) => a - b);
const orderedNumberGroups = order.reduce((acc, key) => {
  const arrayForTheKey = unorderedNumberGroups.get(key);
  return acc.concat([arrayForTheKey]); // note the breakets!

  // equivalent code:
  // acc.push(arrayForTheKey);
  // return acc;
}, []); // initialize the accumulator with an empty Array
console.log(orderedNumberGroups);

Abominate

Thank you for this solution. It certainly works, and looks a lot quicker. But I'm struggling to follow how it works, even with debugger. (newbie). I'll take some time and break it down and see if I can make sense of it as it all looks new and useful – Ungodly

I'm assuming the reduce function and the shortcut with the ? are the problems correct? – Abominate

@VoodooScience I've added a debuggable version that hopefully is better understandable. – Abominate

Yes, I think they were making it very hard. That's really helpful thank you very much! – Ungodly