2

I'm trying to make a function to remove null values from multidimensional arrays, but it doesn't work very well. It does not penetrate to the last layer of arrays, and does not remove when two null values are adjacent.

function isArray(obj) {
    // http://stackoverflow.com/a/1058753/1252748
    return Object.prototype.toString.call(obj) === '[object Array]';
}

function removeEmptyArrayElements(obj) {

    for (key in obj) {
        if (obj[key] === null) {

            obj = obj.splice(key, 1);

        }
        var isArr = isArray(obj[key]);
        if (isArr) {
            removeEmptyArrayElements(obj[key]);
        }

    }
    return obj;
}
KatieK
  • 13,586
  • 17
  • 76
  • 90
1252748
  • 14,597
  • 32
  • 109
  • 229
  • 1
    possible duplicate of [Remove empty elements from an array in Javascript](http://stackoverflow.com/questions/281264/remove-empty-elements-from-an-array-in-javascript) – Baz1nga May 14 '12 at 20:05
  • Better check some Javascript tutorials first ;) Especially the sections about arrays and objects and their differences and how to handle them properly. – Andreas May 14 '12 at 20:07
  • do you have a suggestion on a good place to find those sorts of tutorials? – 1252748 May 14 '12 at 20:45
  • https://developer.mozilla.org/en/JavaScript – Andreas May 15 '12 at 23:16

2 Answers2

7

Don't remove elements from an object while you are iterating over it.

Don't use for ... in on Arrays.

Do take advantage of higher-order functions like filter and map. Try this:

function removeEmptyArrayElements(arr) { 
   if (!isArray(arr)) {
      return arr;
   } else {
       return arr.filter( function(elem) { 
          return elem !== null
       } ).map(removeEmptyArrayElements)
   }
}

Explanation:

"Higher-order" functions are functions that use other functions as values - either accepting them as parameters, or returning them, or both. In this case I used two methods of Javascript's Array class which accept a function as a parameter and use it to build and return a new array.

someArray.filter( function ) calls function on every element of someArray and returns a new array containing only the elements for which function returned true. The function I passed in this case is function(elem) { return elem !== null } - that is, an anonymous function that returns true if the element is not null. So the result of that filter call is the original array with all the nulls removed.

Now we want to remove any nulls from any elements of the result that are themselves arrays. I used map for that. someArray.map( function ) also calls function on every element of the array, but instead of returning a subset of its input based on those results, it just returns the results themselves. For example, [1,2,3].map(function(x){return x*2}) returns the array [2,4,6]. In this case, the function we pass is the one we're in the middle of defining .. a recursive call back to removeEmptyElements.

We do need a way to stop this endless cycle of calling ourselves, and that's what the outer if is for - when called on an element that isn't an array, we just return it as-is.

Mark Reed
  • 91,912
  • 16
  • 138
  • 175
  • it works! I don't know how. But it works! I will need to investigate it a bit.. Thank you very much. – 1252748 May 14 '12 at 20:41
2

You're looping over an array you are deleting from. This gives you the "does not remove when two null values are adjacent" bug. The "It does not penetrate to the last layer of arrays" part is probably because you're not reassigning the value at obj[key]. Arrays are copied by value, not referenced.

I'd do it like this:

function isArray(obj) {
    // http://stackoverflow.com/a/1058753/1252748
    return Object.prototype.toString.call(obj) === '[object Array]';
}

function removeEmptyArrayElements(arr) {
    var i;
    for (i = 0; i < arr.length; i += 1) {
        if (arr[i] === null) {
            arr = arr.splice(i, 1);
            i -= 1; // decrement counter!

        } else if (isArray(arr[i])) {
            arr[i] = removeEmptyArrayElements(arr[i]);
        }
    }
    return arr;
}
Halcyon
  • 57,230
  • 10
  • 89
  • 128
  • thanks! you are still splicing with key though, which isn't defined. I tried replacing with i, but created something that crashed my browser : ( – 1252748 May 14 '12 at 20:39
  • Hmm, maybe the splice doesn't work right and ends up in a loop? Check the length of the array after splicing, it should have gone down by 1. – Halcyon May 14 '12 at 20:41