Removing properties and elements set to null in JavaScript I'm looking for a review of the following code.


Is there a better way to remove all properties that evaluate to truthy for a given function?
Is there a better way to prevent a stack overflow in the recursive function?


This is the entire Node.js module:

// The removePropertiesWhereTruthy function accepts the following parameters:
// 
// 1. an object or array
// 2. a function to call against each non-object or non-array to determine if it's a truthy value according to that function (e.g. _.isNull)
// 3. An optional maxRecursion value to prevent a stack-overflow when there's a circular reference or exceptionally deeply nested objects.
// 
'use strict';

var _  = require('lodash');

function removePropertiesWhereTruthy(obj,func,maxRecursion) {

  maxRecursion = _.isNumber(maxRecursion) ? maxRecursion - 1 : 20;
  if(maxRecursion &lt; 0) {
    // prevent stack overflow by allowing caller to set a max time for
    // recursion and default to 20 if not set.
    return;
  }
  if(_.isObject(obj) &amp;&amp; !_.isArray(obj)) {
    _.forOwn(obj, function(v, k) {
      if(_.isObject(v)) { // also true for array
        return removePropertiesWhereTruthy(v,func,maxRecursion);
      } else if(func(v)) {
        delete obj[k];
      }
    });
  } else if(_.isArray(obj)) {
    _.remove(obj, function(item){
      return func(item);
    });
    _.each(obj, function(v){
      if(_.isObject(v)) { // also true for (nested) array
        return removePropertiesWhereTruthy(v,func,maxRecursion);
      }
    });
  } else {
    throw new Error('This function should only be called with objects and arrays');
  }
}

function removePropertiesWithNullValues(obj, maxRecursion) {
  return removePropertiesWhereTruthy(obj, _.isNull, maxRecursion);
}

module.exports = {
  removePropertiesWithNullValues: removePropertiesWithNullValues
};
I can't think of any good use-case for this method. It may help to describe some examples of why you need this functionality.

That said, a few issues I see:


func could be named better (predicate perhaps?).
maxRecursion seems a bit unstable. This is where I have a hard time seeing the purpose of this method. What happens if you hit level 20 and it hasn't fully run it's course? Now you've got an object that is in a half-complete state without any way to know it. What if it spends 20 cycles on the first property which happens to be a circular reference?
I think the use of lodash here adds confusion to what would otherwise be pretty straightforward javascript (opinion).


Some fixes:


Don't use a recursive function. You could use a stack or a queue here to traverse the object tree.
Rather than limit the depth to avoid stack overflows for circular references, you could try to detect them by keeping a table of references and avoid re-traversing those trees altogether.
Rather than modifying an existing object, you could create a new one. That way, this method wouldn't produce any side-effects and potentially leave you in an unstable state. At minimum, you could return true/false if the method succeeded.