dtex / j5e

Framework for embedded devices using ECMA-419, the ECMAScript® embedded systems API specification, based on Johnny-Five's API
https://www.j5e.dev/
MIT License
64 stars 6 forks source link

Use of Object.defineProperties instead of getters? #73

Closed phoddie closed 4 years ago

phoddie commented 4 years ago

I'm curious why j5e uses Object.defineProperties to define getters.

Here's a typical example in thee switch class:

https://github.com/dtex/j5e/blob/3390ab33e1b55b7c89ceed58a6bc616aa97fafca/lib/switch/index.js#L57-L68

This works, but it has both a memory and a time cost. For each instance, two arrow function objects are instantiated and set. In addition, the temporary object passed to Object.defineProperties is created and eventually garbage collected. Using getters directly would seem to provide the same API without incremental memory cost for each instance or added execution time.

  get isClosed() {
    return Boolean(this.io.read()); 
  }
  get isOpen() {
    return !Boolean(this.io.read()); 
  }
dtex commented 4 years ago

Neat, I'll look at refactoring those. Thanks for the tip!

dtex commented 4 years ago

Well, I thought I'd be smart and try an A/B test to see the improvement, but I couldn't perceive a difference when checking values through XSBug. The code is definitely more readable, and I like that the accessor functions have more flexibility in the documentation when using JSDoc which makes me happy. I've made updates to the code throughout.

phoddie commented 4 years ago

The changes look great. (There are a lot of them!)

I thought I'd be smart and try an A/B test to see the improvement, but I couldn't perceive a difference when checking values through XSBug

The performance improvement of instantiating will be hard to measure on a human scale. It is there. The memory use reduction should be visible in xsbug instrumentation, I believe in the slot memory heap count.