Get collectionChanges on setData

[jamescmartinez]

Let's say I have a network request that returns, and I call:

dataProvider.setData(data, cacheKey: cacheKey)

I expect collectionDataProviderHasUpdatedData to be called, but it isn't. How else should I get the collectionChanges?

Right now, I'm updating my UI in three separate places instead of one: fetchDataFromCache (on view load), setData (after network requests), and collectionDataProviderHasUpdatedData. 😵

[plivesey]

This has been a source of debate internally. The old way I had the API, it always called the delegate method and did not call the completion block. People hated this saying: "this doesn't make sense...I want it to call the completion block instead of the delegate". I agreed with this, so we changed it.

However, I have thought about this and think that maybe it's good to offer this flexibility? But a few questions:

In your network request code, you could just call the delegate yourself right? Would this solve your problem?
How would you feel about adding an additional parameter to setData: updateDelegate: Bool = false
For the collection changes, the change reported will be .reset because you're setting entirely new data. The library currently doesn't support 'minimal change sets'. I have thought about adding this but feel like it probably it a bit out of scope. You should check out https://github.com/osteslag/Changeset though and see if it's something you're interested in.

[jamescmartinez]

@plivesey Oh! Now I understand. I figured there would be a single source of notification (the delegate). The current implementation makes sense too, though!

To answer your questions:

• I can totally call the delegate myself and, in fact, that's what I've done to get around this problem. 😄
• An option to setData would indeed solve the problem, but if all it's doing is calling the delegate with .reset then maybe this option is overkill.

I might be the only one that expected a notification, so I'd keep the current implementation for now. Maybe additional documentation would help clear this up for others in the future?

Also, the library you linked looks promising! I have some array diffing code that I wrote, but might as well use this lib instead. 👍

Thanks @plivesey!!

[akaralar]

Actually now reading about it, initially I also thought that the delegate will be called after the first fetch. Having 2 places to get the same data seems redundant, but at the same time it might be useful to differentiate between an object that comes from a fetch and an object that comes from an update to the model. So one way to deal with this might be to have an enum

enum UpdateType {
    case .fetch
    case .update 
}

and include this enum in the delegate method as a parameter as well, and you wouldn't need to have a completion block for a fetchDataFromCache. Or you can have both this and a fetchDataFromCache method with an update block, whose parameters are the same as the delegate method, and you don't need to set a delegate and each update just calls that block.

Generally I vote for a single method to deliver all fetches and updates for a data provider (be it blocks or delegates), and a way to differentiate whether the data is coming from a fetch (ie cache) or an outside update to the model.

[plivesey]

This was similar to our original API, but it makes writing caching and networking code difficult. In theory it sounded great, but it was pretty unanimously disliked. For instance, this code gets really hard to write:

spinner.startSpinning()
dataProvider.fetchFromCache(args) { _ in
    tableView.reloadData()
    spinner.stopSpinning()
}

To do this fully correctly with a delegate method, you need to pass in a context and check that which is much uglier in code.

If you want this functionality, you can subclass DataProvider yourself and provide your own delegate and implementation of these methods, but for the public API of this library, I prefer what we have now.

[akaralar]

Makes sense, thanks for the explanation!