Skip to content

Conversation

@plivesey
Copy link
Owner

No description provided.

@plivesey
Copy link
Owner Author

API changes:

// protocol Model and protocol SimpleModel

func isEqualToModel(_ model: Model) -> Bool {}
func isEqual(to model: Model) -> Bool {}

func mergeModel(_ model: Model) -> Model
func merge(_ model: Model) -> Model

// DataProvider, CollectionDataProvider, BatchDataProviderListener

open var paused: Bool
open var isPaused: Bool

// DataProvider, CollectionDataProvider

open func fetchDataFromCache(cacheKey cacheKey: String?, ...)
open func fetchDataFromCache(withCacheKey cacheKey: String?, ...)

// CollectionDataProvider

open func removeAtIndex(_ index: Int, shouldCache: Bool = true, context: Any? = nil)
open func remove(at index: Int, shouldCache: Bool = true, context: Any? = nil)

Please also pay attention to all the APIs that I did not change.

@plivesey plivesey mentioned this pull request Sep 22, 2016
@codecov-io
Copy link

codecov-io commented Sep 22, 2016

Current coverage is 90.01% (diff: 94.81%)

Merging #39 into master will decrease coverage by 0.06%

@@             master        #39   diff @@
==========================================
  Files            43         43          
  Lines          4538       4538          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           4088       4085     -3   
- Misses          450        453     +3   
  Partials          0          0          

Powered by Codecov. Last update 95376f2...2704840

At this point, the data provider will already have new data, so there's no need to call setData.
*/
open func fetchDataFromCache(cacheKey: String?, context: Any? = nil, completion: @escaping ([T]?, NSError?)->()) {
open func fetchDataFromCache(withCacheKey cacheKey: String?, context: Any? = nil, completion: @escaping ([T]?, NSError?)->()) {
Copy link
Contributor

@nicksnyder nicksnyder Oct 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to consider:
fetchCachedData(withKey cacheKey: String?, ...)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm...originally liked, but maybe not...I think I prefer cache key because it's consistent naming across different places?

- parameter context: This context will be passed onto the cache delegate. Default nil.
*/
open func removeAtIndex(_ index: Int, shouldCache: Bool = true, context: Any? = nil) {
open func remove(at index: Int, shouldCache: Bool = true, context: Any? = nil) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if this should have a noun

removeItem(at index: Int, ...)
removeData(at index: Int, ...)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so based off Apple's APIs. Apple specifically suggests against this in API guidelines https://swift.org/documentation/api-design-guidelines/ (Omit needless words)

from ^
bad public mutating func removeElement(_ member: Element) -> Element?
good public mutating func remove(_ member: Element) -> Element?

@plivesey plivesey merged commit 65597af into master Oct 3, 2016
@plivesey plivesey deleted the swift3api branch October 3, 2016 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants