rockandrollwithemberjs / rarwe-issues

Errata and updates for the Rock and Roll with Ember.js book
https://rockandrollwithemberjs.com
43 stars 4 forks source link

[Chapter 8] Remove some magic when referencing controller storage property #491

Closed GradedJestRisk closed 4 years ago

GradedJestRisk commented 4 years ago

In catalog.js, storage has 2 properties: bands and songs

@tracked storage = {}; constructor() { super(...arguments); this.storage.bands = []; this.storage.songs = []; }

When we add a record to them, storage property is omitted

add(type, record) { let collection = type === 'band' ? this.bands : this.songs;

That works, but it's kind of weird (unless you mention it) So, I suggest moving out the magic and reference storage explicitly

add(type, record) { let collection = type === 'band' ? this.storage.bands : this.storage.songs;

balinterdi commented 4 years ago

Hmm, I'd like to understand better what seems magical about it.

The point of defining a getter is to create an alias to the underlying storage mechanism. Furthermore, it provides a great way to hide the underlying implementation from users of the service, separating the private and public API of the class.

For example, let's assume bands and songs can be archived and we don't want to return those to the outside world. Having a bands getter, we could just filter out the archived ones whereas if we return this.storage.bands we can't do that.

GradedJestRisk commented 4 years ago

We'll, you're right to look for clarification, I'm okay with the bands getter and encapsulation in OOP The getter internal reference storage return this.storage.bands;

 get bands() {
 return this.storage.bands;
 }
``
`
What I want to point to is the setter below, which doesn't reference `storage` explicitly
If description above isn't clear, let me know.

add(type, record) { let collection = type === 'band' ? this.bands : this.songs;

balinterdi commented 4 years ago

Ah, that's a great point, I've just fixed this. Thank you!