Chapter 4: Abstraction Example wrong logic

[vesheff]

In the last abstraction example (when you took the abstraction too far) the logic in the code is wrong compared with the original example shown earlier. So if this is intentionally, please disregard the comment below and just add clarification in the description of the example.

1. First mistake

   function isPropUndefined(val,obj,prop) {
       return isUndefined( obj[prop] );
   }

• The code above is ok to check if a property of an object is undefined, but here you change the logic of the previous example and check if a property of the store is undefined.

  • Original logic:

  if (evt.name !== undefined) {
        storeData( events, evt.name, evt );
  }

  2. Second mistake

• You return isUndefined result which is true if the property is undefined, so the logic is wrong once again.

Proposal:

• Just change the name of the function and wrap the function in another one which accepts the property to be checked against undefined. This will lead to the need of invoking the function with an argument name in this case.

  function isPropDefined(prop) {
      return function (obj) {
          return !isUndefined(obj[prop]);
      }
  }
  
  function trackEvent(evt) {
      conditionallyStoreData(events, evt.name, evt, isPropDefined('name'));
  }

• Now we could change the invokation of checkFn to be with only one argument.

  if (checkFn(value)) {
      store[location] = value;
  }

[getify]

The code above is ok to check if a property of an object is undefined, but here you change the logic of the previous example and check if a property of the store is undefined.

"property of an object" vs "property of the store"... these are the same thing, I'm not sure what distinction you're trying to make?

You return isUndefined

You're correct, I accidentally reversed the logic here. Not that it really matters all that much, since this whole section of code is what not to do. But it's something that could be cleaner. The simple fix would just be:

function isUndefined(val) { return val === undefined; }

function isPropDefined(val,obj,prop) {
   return !isUndefined( obj[prop] );
}

It's something I'll take a look at for next revisions/editions of the book.

[vesheff]

"property of an object" vs "property of the store"... these are the same thing, I'm not sure what distinction you're trying to make?

You are right. The thing is that in the original example you check if the name of the event is undefined, but in the last example you check if the key in the store is undefined, so this is the wrong logic I'm trying to fix.

const events = {
    'js-conf': {
        name: 'js-conf'
    }
};

trackEvent({});

console.log(events); 
// Example 1: { 'js-conf': { name: 'js-conf' } }
// Example 2: { 'js-conf': { name: 'js-conf' }, undefined: {} }

[getify]

Oh, I see. OK, yes, I'll fix that too.

[vesheff]

If you want I could make a pull request on that.

[getify]

No thanks. I won't be making changes to the book since it's been in print for almost 2 years now. These changes will be addressed when/if I release a second edition.