Eloquent Javascript Exercise: Deep Comparison

Question

I'm learning JavaScript and trying to do this exercise:

Write a function deepEqual that takes two values and returns true only if they are the same value or are objects with the same properties, where the values of the properties are equal when compared with a recursive call to deepEqual.

To find out whether values should be compared directly (use the === operator for that) or have their properties compared, you can use the typeof operator. If it produces "object" for both values, you should do a deep comparison. But you have to take one silly exception into account: because of a historical accident, typeof null also produces "object".

The Object.keys function will be useful when you need to go over the properties of objects to compare them.

This is my solution and gives me true, false, false which is wrong

function deepEqual (a,b) {
    if (a===b) return true;
    if (typeof a=="object" && typeof b=="object") {         
        let x=Object.keys(a), y=Object.keys(b);
        if (x.lenght==y.lenght) {
            for (key of x){
                if (y.includes(key)){
                    if (a[key]===b[key]) return true;
                    else return false;
                }
            }
        }
        return true;
    } 
}

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

This is the right solution:

function deepEqual(a, b) {
    if (a === b) return true;
    if (a == null || typeof a != "object" ||
        b == null || typeof b != "object") return false;
    let keysA = Object.keys(a), keysB = Object.keys(b);
    if (keysA.length != keysB.length) return false;
    for (let key of keysA) {
        if (!keysB.includes(key) || !deepEqual(a[key], b[key])) return false;
    }
    return true;
}


let obj = {here: {is: "an"}, object: 2};
console.log(deepEqual(obj, obj));
// → true
console.log(deepEqual(obj, {here: 1, object: 2}));
// → false
console.log(deepEqual(obj, {here: {is: "an"}, object: 2}));
// → true

I can't really understand what's wrong with what I wrote and how can I fix it.

Thank you very much for helping a noobie :)

Answer 1

Here are the issues in your code:

1) The test for objects:

if (typeof a=="object" && typeof b=="object") {

This is fine, but what if this condition is not true? You don't have any code that deals with that case, so the function will return undefined. It should return false instead.

Secondly, there is this weird thing in JavaScript about which the question has warned you -- you should have taken it into account:

...because of a historical accident, typeof null also produces "object".

...so you need to have a separate test for that, as null is not really an object and code will produce errors when you treat it as one.

In your code a null value would pass this if test and would then execute Object.keys(null), which triggers an exception. A null value should not be allowed in there.

2) Comparing the number of properties

    if (x.lenght==y.lenght) {

There is a spelling mistake. It is not lenght, but length (2x). And again, what if this condition is not true? There is no code that deals with that case, except for the return true after that if block, yet your function should then return false.

3) Check that a property exists in both objects

            if (y.includes(key)){

This test works, but is not optimal. includes is slower than just checking that key in b (it is a pity that the reference solution also uses includes).

And as before, what if this condition is not true? The function should immediately exit the loop and return false, but this is not happening...

4) Check that the values for the same properties are deep equal.

                if (a[key]===b[key]) return true;

This condition does not do a deep comparison. The question already told you what to do here:

...where the values of the properties are equal when compared with a recursive call to deepEqual

So here is that occasion where you should perform a recursive call, so any nested objects are compared in the same way. Just a[key]===b[key] will be false when these two values are separate objects. But the condition should really be true when those objects have the same properties, and same values for them... which is exactly what your function is able to do: so call it here.

Secondly, this is not the time to return true, as this will exit the loop without checking also the other keys, and there the outcome could be negative... And one "negative" is enough to make the overall end result false. So while things are good, you should continue the loop.

5) When the values are different

                else return false;

Yes, this is the right time to return false.

6) When all tests succeeded

    return true;

This is the only right place to return true, but you should solve the second remark in my answer so this statement is not executed when the lengths are different.

The correct code

As you already have correct code to compare with, it is a bit overkill to repeat that code here.