Modified deepEqual function doesn't work

Question

I was looking at the deepEqual comparison function from the Eloquent Javascript exercises. I thought I could make an improvement on the suggest solution by moving the if statement that checks for the object existence and value equality check to the first for..in loop rather than the second.

My reasoning was that it would allow the check to fail earlier if the objects didn't have matching properties or their values were different rather than waiting to do so in the second loop.

The change did not work. This jsbin demonstrates the issue, and this is the code:

function deepEqual(a, b){
  if(a === b) return true;
  if(a === null || typeof a !== "object" ||
     b === null || typeof b !== "object")
    return false;

  var pA = pB = 0;
  //console.log('OBJECT detected vals:',a,b);
  for(var p in a){
    pA++;
    //console.log('pA:'+pA, p, a, (p in b));
    // MOVED THE IF STATEMENT INTO THIS LOOP INSTEAD OF THE
    // SECOND LOOP BELOW
    if(!(p in b) || !deepEqual(a[p], b[p]))  
      return false;
  }

  for(var p in b){
    pB++;
    //console.log('pB:'+pB, p, b, (p in a));
    //if(!(p in a) || !deepEqual(a[p], b[p]))
      //return false;
  }

  return pA === pB;
}

var obj = {here: {is: "an"}, object: 2};
console.log(deepEqual(obj, obj));
// → true WORKS
console.log(deepEqual(obj, {here: 1, object: 2}));
// → false WORKS
console.log(deepEqual(obj, {here: {is: "an"}, object: 2}));
// → true, DOES NOT WORK, LOGS OUT FALSE...?

Commenting out the console.log call in snippet should render the following output:

console.log(deepEqual(obj, {here: {is: "an"}, object: 2}));
OBJECT detected vals: Object {here: Object, object: 2} Object {here: Object, object: 2}
pA:1 here Object {here: Object, object: 2} true
OBJECT detected vals: Object {is: "an"} Object {is: "an"}
pA:1 is Object {is: "an"} true
pB:1 is Object {is: "an"} true <-- why is this being called here, pA loop hasn't finished???
pA:2 object Object {here: Object, object: 2} true
pB:2 here Object {here: Object, object: 2} true
pB:3 object Object {here: Object, object: 2} true

From what I can see, the for..in for the b argument starts early here and affects the later running of the loop with correct values.

What have I missed?

---

The way I think it should work

As I understand it, the 3rd log should run as follows:

CALL:

deepEqual({here: {is: "an"}, object: 2},{here: {is: "an"}, object: 2})

• (a === b)→ false,
• a and b are objects, continue
• set pA and pB to 0

• start for..in loop

  1. pA is 1, p is here
     • here is in b so have to CALL:
     • deepEqual({is: "an"}, {is: "an"})
     • they're both objects so go:
     • start for..in loop
       • pA is 1, p is is
         • here is in b so have to CALL:
         • deepEqual("an", "an")
     • <<<<<<<<<<<< RETURN TRUE!!!
     • if fails, start next iteration of loop:
  2. pA is 2, p is object
     • object is in b so have to CALL:
     • deepEqual(2, 2)
     • they're both objects so go:
     • start for..in loop
       • pA is 1, p is is
         • here is in b so have to CALL:
         • deepEqual(a['is'], b['is'])
     • <<<<<<<<<<<< RETURN TRUE!!!

• At this point pA is 2

• All that's left to do is, iterate over props in b

• pB should be 2

Return (pA === pB), which is the same as return 2===2, which is return true

It's currently logging out false.

Answer 1

You've been bitten by JavaScript's Implicit Globals

The problem is this line:

var pA = pB = 0;

When deconstructed into multiple lines, it looks like:

pB = 0;
var pA = pB;

Which means pB is not getting declared with var and is thus a global variable, not a variable local to deepEqual. This means it preserves its value through calculations and hence has the wrong value.

Your function returns false because pB for the top object level with 2 properties ends up with a value 3 instead of 2 because it remembers its count from the inner level, which had 1 property { is: "an" }. The other two tests work because they only check a single level of properties.

If you make pB a local variable everything works:

function deepEqual(a, b) {
  if (a === b) return true;
  if (a === null || typeof a !== "object" ||
    b === null || typeof b !== "object")
    return false;

  // make both variables local
  var pA = 0;
  var pB = 0;
  for (var p in a) {
    pA++;
    if (!(p in b) || !deepEqual(a[p], b[p]))
      return false;
  }

  for (var p in b) {
    pB++;
  }

  return pA === pB;
}

var obj = {here: {is: "an"}, object: 2};
console.log(deepEqual(obj, {here: {is: "an"}, object: 2}));

Here's a related question worth checking out.