0

I have 2 arrays - collections and settings. IN collections array I need to remove empty arrays but the same index I need to remove from array settings so I write:

$.each(collections, function(index, collection) {
        if (collection.length == 0) { 
          collections.splice(index, 1); 
          settings.splice(index, 1);
        }
    });

It works only for the first empty array but if there is more than 1 empty array I got error message:

Uncaught TypeError: Cannot read properties of undefined (reading 'length')

How to remove empty arrays from collections but at the same time to remove the same index from settings array?

Aleks Per
  • 1,549
  • 7
  • 33
  • 68
  • Did you try using `filter()` method? – Beni Sinca Jun 06 '22 at 11:19
  • why not just `collections.filter( cl => cl.length !==0 )` ? – AdityaParab Jun 06 '22 at 11:19
  • Yes, I try this code ollections = collections.filter(e => e.length); and works fine but how to remove index from settings array too – Aleks Per Jun 06 '22 at 11:20
  • Loop through your array in reverse order using a standard for loop, or use `$.filter()`. The `$.each()` loop will loop over the original array length, and modifying your array by removing elements shrinks the array length, but doesn't change the number of iterations you do. As a result, you end up iterating over indexes that no longer exist in your array and you will also skip elements that shift into indexes that have already been visited by the loop. – Nick Parsons Jun 06 '22 at 11:28

2 Answers2

1

Just do a truthy check before you access the length:

if (!collection || collections.length == 0) { ... }

(this will short-circuit if collection is falsy (undefined is falsy) and thus no error will be thrown).

I'm not sure what ramifications the modification of the array within $.each is having, but you could also just do this:

let collectionsCopy = collections;

$.each(collectionsCopy, function(index, collection) {
    if (!collection || collection.length == 0) { 
        collections.splice(index, 1); 
        settings.splice(index, 1);
    }
});

(i.e. create an array whose only purpose is to loop over collections with - massively inefficient but this will work and is not dissimilar from your original approach.)

To make a more effective solution you could also just store the indices you want removed:

let indices = [];

$.each(collections, function(index, collection) {
    if (!collection || collection.length == 0) {
        indices.push(index);
    }
});

$.each(indices, function(_, index) {
    collections.splice(index, 1);
    settings.splice(index, 1);
});

If you'd like to use filter:

collections = collections.filter((collection, index) => {
    if (!collection || collection.length == 0) {
        settings.splice(index, 1);
        return false;
    }
    return true;
});
Jack Bashford
  • 43,180
  • 11
  • 50
  • 79
  • $.each(collections, function(index, collection) { if (collection && collection.length == 0) { collections.splice(index, 1); settings.splice(index, 1); } }); this code does remove only 1st empty array and ignore all other empty arrays – Aleks Per Jun 06 '22 at 11:18
  • @AleksPer I can am fairly certain that's not the case. If the element is `undefined` or an object with `length = 0` as a property (i.e. an empty array) then the `if` statement will be matched. You may be having issues if `$.each` uses the array as it is modifying itself. Perhaps use a duplicate and don't modify `collection` while you're looping through it? (I'm not exactly sure what problems this will cause). – Jack Bashford Jun 06 '22 at 11:24
  • here is fiddle https://jsfiddle.net/hynd5xs9/ – Aleks Per Jun 06 '22 at 11:25
  • @AleksPer see my updated answer. – Jack Bashford Jun 06 '22 at 11:27
  • How I can use with .filter ? – Aleks Per Jun 06 '22 at 11:27
  • 1
    @AleksPer added a `filter` solution which should work. – Jack Bashford Jun 06 '22 at 11:32
  • I think changes need in let indices = []; $.each(collections, function(index, collection) { if (collection && collection.length == 0) { indices.push(index); } }); $.each(indices, function(_, index) { collections.splice(index, 1); settings.splice(index, 1); }); because we want position (values) from indices not index ... – Aleks Per Jun 06 '22 at 11:32
  • Please test on jsfiddle all your solutions with this values var collections = [ [1], [], [], [2], [] ]; var settings = [ [5], [6], [7], [8] ]; and you will see that is not correct – Aleks Per Jun 06 '22 at 11:36
  • @AleksPer every time a `.splice()` occurs, the indexes will be off by one. This needs to be accounted for, eg: by pushing `indices.push(index-indices.length);`, will offset each index by the number of splices/shifts that will occur before it, allowing you to remove the correct element (although I think looping in reverse order makes things a bit clearer & avoids this particular casee) – Nick Parsons Jun 06 '22 at 11:52
1

Try this:

var collections = [
  [1],
  [],
  [],
  [2],
  []
];
var settings = [
  [5],
  [6],
  [7]
];

collections.forEach(function(collection, index) {
  if (collection.length === 0) {
    collections[index] = null;
    if (settings[index]) {
      settings[index] = null;
    }
  }
})

// then filter out the nulls
collections = collections.filter(function (v) { return v !== null });
settings = settings.filter(function (v) { return v !== null });

console.log('cols:', collections, 'setts:', settings); // cols:", [[1], [2]], "setts:", [[5], [8]]

fully working example is there: https://jsfiddle.net/5vfbdowL/1/

Andy
  • 523
  • 2
  • 13
  • https://jsfiddle.net/hynd5xs9/ please check, as you can see still empty arrays in array ... – Aleks Per Jun 06 '22 at 11:23
  • just updated version: https://jsfiddle.net/5vfbdowL/1/. The better way is not to change the size inside the loop fn since we loose index rather mark elements and then filter out them safely – Andy Jun 06 '22 at 12:45