2
\$\begingroup\$

I created a simple Javascript method to recursively parse a JS Object and find out if any of its values or sub values is set to null or is empty string.

I could't break on when using Array.map() or Array.forEach() so I went with this :

const isValidObject = (obj) => {
  let invalid = []
  const validate = (obj) => {
    Object.values(obj).map(el => {
      if (typeof el === 'object') {
        validate(el)
      } else if (el === null || el === '') {
        invalid.push(el)
      }
    })
    return invalid.length === 0
  }
  return validate(obj)
}

If you have any opinonated or objective thought about this way of doing, please share it with me.

const isValidObject = (obj) => {
  let invalid = []
  const validate = (obj) => {
    Object.values(obj).map(el => {
      if (typeof el === 'object') {
        validate(el)
      } else if (el === null || el === '') {
        invalid.push(el)
      }
    })
    return invalid.length === 0
  }
  return validate(obj)
}

let toto = {
	name: 'toto',
  age: 12,
  pony: {
    name: 'rainbow dash',
    age: ''
  }
}

console.log('test :', isValidObject(toto))

\$\endgroup\$
1
\$\begingroup\$

Testing

Technically your question is against rules as it is not a working example, and I wonder if you ran it at all

See what happens when you run it.

const isValidObject = (obj) => {
  let invalid = []
  const validate = (obj) => {
    Object.values(obj).map(el => {
      if (typeof el === 'object') {
        validate(el)
      } else if (el === null || el === '') {
        invalid.push(el)
      }
    })
    return invalid.length === 0
  }
  return validate(obj)
}

const test = {c : 1, d : null};

console.log("test : " + isValidObject(test))

The reason is that null is of type "object"

console.log(typeof null)

So you would need to rearrange the tests a little testing for null before you test for typeof obj === "object"

const isValidObject = (obj) => {
  let invalid = []
  const validate = (obj) => {
    Object.values(obj).map(el => {
      if (el === null || el === '') {  // Moved this statement up 
        invalid.push(el)
      } else if (typeof el === 'object') {
        validate(el)
      }
    })
    return invalid.length === 0
  }
  return validate(obj)
}

const test = {c : 1, d : null};

console.log("test : " + isValidObject(test))

Cyclic references.

But still the approch is unfortunately flawed.

See what happens in the next example

const isValidObject = (obj) => {
  let invalid = []
  const validate = (obj) => {
    Object.values(obj).map(el => {
      if (el === null || el === '') {  // Moved this statement up 
        invalid.push(el)
      } else if (typeof el === 'object') {
        validate(el)
      }
    })
    return invalid.length === 0
  }
  return validate(obj)
}

const test = {a : 1, b : 2};
const test1 = {a : 1, b : 2, c : test};
test.c = test1;

try {
   console.log("test : " + isValidObject(test))
   console.log("test1 : " + isValidObject(test1))
}catch(e){
   console.log("Catching the error.");
   console.log(e.message);
}
<div id="myDiv">Hi... :)</div>

The cause is that you have entered a cyclic recursion. test object references test1 which references test and so on and on ...

Objects like this are called cyclic objects and it is very common to have cyclic objects, nor is it a bad practice to use cyclic object.

There are two ways to combat the problem.

  • Count iteration depth and set a limit. Hacky but works.
  • Keep a Set of objects you have processed and check that you are not testing the same object more than once.

const isValidObject = (obj) => {
  const tested = new Set();
  const invalid = [];
  const validate = (obj) => {
    Object.values(obj).map(el => {
      if (el === null || el === '') {  // Moved this statement up 
        invalid.push(el);
      } else if (typeof el === 'object') {
        if(!tested.has(el)){
            tested.add(el);
            validate(el);
        }
      }
    })
    return invalid.length === 0;
  }
  return validate(obj);
}

const test = {a : 1, b : 2};
const test1 = {a : 1, b : 2, c : test};
test.c = test1;


console.log("test : " + isValidObject(test))
console.log("test1 : " + isValidObject(test1))

That protects you from cyclic references.

Avoid recursion

Because it is impossible to know the current call stack depth recursion in JS is fundamentally flawed. You should avoid using recursion if you can.

In this case using recursion is a just creating (an expensive) stack for you by holding the previous objects on the heap. You push to the heap with a function call and you pop from the heap with a return.

Old fashioned loops are still important.

Seams everyone is transfixed on having to use the array iteration methods. One must remember that they carry with them a considerable overhead. This is even more important when you are using recursion to solve the problem. Each time you call a function you create a new context that is added to the heap.

Because your function does not fall out of the recursion until the deepest iteration the heap usage can be quite large. You would not want to test a linked list of any size. Because you are using a callback to iterate the object you actually need to create two heap allocations for ever one object you step into.

The net effect is that this function will keep GC busy and degrading the quality of the user experience.

This can be done without recursion, but you need to replace the heap with your own stack. This results in efficient use of memory as you only need one reference for each object you step into rather than a complete context (2 context in your case).

I have created two versions.

  1. Using Array.some to find the null or empty string and then Array.filter and Array.push to add objects to the stack.

  2. for of loop to iterate the values of each object. Pushing to the stack objects as needed.

function isValidObject(obj) {
    const stack = [obj];
    const tested = new Set();
    tested.add(obj);

    while (stack.length > 0) {
        const o = Object.values(stack.shift());
        if (o.some(val => val === null || val === "")) { return false }
        stack.push(...o.filter(val => {
            if (typeof val === "object" && !tested.has(val)) {
                tested.add(val);
                return true;
            }
            return false;
        }));

    };
    return true;      
}

// or even faster and less GC impact

function isValidObject_1(obj) {
    const stack = [obj];
    const tested = new Set();
    tested.add(obj);
    while (stack.length > 0) {
        for (const val of Object.values(stack.shift())) {
            if (val === null || val === "") { return false }
            if (typeof val === "object" && !tested.has(val)) {
                tested.add(val);
                stack.push(val);
            }
        }
    }
    return true;      
}


const test = {a : 1, b : 2};
const test1 = {a : 1, b : 2, c : test};
const test2 = {a : 1, b : 2, c : test, d : null};
const test3 = {a : 1, b : 2,  d : ""};
test.c = test1;


console.log("Test first version.");
console.log("test : " + isValidObject(test));
console.log("test1 : " + isValidObject(test1));    
console.log("test2 : " + isValidObject(test2));    
console.log("test3 : " + isValidObject(test3));    

console.log("Test second version.");
console.log("test : " +  isValidObject_1(test));
console.log("test1 : " + isValidObject_1(test1));    
console.log("test2 : " + isValidObject_1(test2));    
console.log("test3 : " + isValidObject_1(test3));

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for this complete and very helpful answer, accepted :) \$\endgroup\$ – Neovea Jan 11 '18 at 9:59
1
\$\begingroup\$

Say hello to your new friend, Array.prototype.every. You'll have a callback which actually returns false on any element which is null or ''. The .every call will be short-circuited and your iteration won't have to go over all the nested elements. No need for extraneous invalid array either.

\$\endgroup\$
3
  • \$\begingroup\$ this is alright, this is fine if I dont make it recursive : if typeof el is object, it always returns false and doesn't go back through my validate function jsfiddle.net/9drdo7oa/1 \$\endgroup\$ – Neovea Jan 10 '18 at 16:26
  • \$\begingroup\$ @Neovea You need to return validate(el) See jsfiddle.net/9drdo7oa/2 \$\endgroup\$ – hjpotter92 Jan 10 '18 at 16:38
  • \$\begingroup\$ Like the straightforward style of this answer, but the other one is more reliable to me. Thanks I learned something indeed. \$\endgroup\$ – Neovea Jan 11 '18 at 9:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.