Refactoring discussion

[jonesetc]

@ksnyde I'm going to use this as a spot to jot down some notes as I go through code. I'm probably about to have a few days where I won't be able to act on these things, so I'd like to get my thoughts down so you can comment and decide if you want to keep contributing or if the approach I want is too at odds for you.

Some high level notes before I get into specifics:

• I want there to be two distinct adapters, fetch and watch. Mixing them together makes things weird and I believe there is a reason they are separated in Horizons API. Somethings don't need to be real-time so it shouldn't be forced. But it should be easy enough that just swapping adapters will make it automatically just work.
• The adapter should handle the connecting of Horizon and the store, this is the intent of the adapter and right now I feel like too much of this logic is being pulled into the service.
• Following with this, I think the service should also stay relatively lightweight and not try to do too much. Part of the reason that this is not called ember-cli-horizon-adapter is that I would like this to be useful to people who just want to open a connection and not include any of the ember-data pieces. Having the service do too much bookeeping and cleanup of its own is going to lead to restricting the usage of that service.

[jonesetc]

https://github.com/jonesetc/ember-cli-horizon/blob/c29ef3997b295a6022001d4b632d411b598f5185/addon/services/horizon.js#L22

This is a bad sign. One, it's a smell that we've broken the specialization that the adapter is supposed to have. Two, there is a reason that the adapter is handed the store with every method. This addon should never rely on an injected store. My other addon is a multi-store-service, I've been bitten by the injected store before.

[yankeeinlondon]

Ok, wasn't in love with store being there either; thanks for pointing it out.

[yankeeinlondon]

WRT to two distinct adapters, I feel this might limit flexibility. I'm 100% with you that "real-time" isn't always a requirement so my thinking was that by default it would be off but an application could register interest in real-time on a collection-by-collection basis. At the point that this interest is registered then the adapter's read operations would just move to "peek" based operations because the real-time updates would ensure that the store is up-to-date.

Does this make sense?

[yankeeinlondon]

Not sure I'm understanding your view regarding service versus adapter concerns. Your point about not having people have to sign up for Ember-Data is my goal as well. In fact, I'd very much like to add support for Ember-Redux at some future point. When you have a moment today maybe we can chat about this aspect so I can understand better where your head is at.