Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: Completion handlers are called in the original threads #40

Closed
Hengyu opened this issue May 11, 2016 · 4 comments
Closed

Question: Completion handlers are called in the original threads #40

Hengyu opened this issue May 11, 2016 · 4 comments

Comments

@Hengyu
Copy link

Hengyu commented May 11, 2016

The completions in class which conforms to StorageAware is called in its original thread. For example, the completion of a add method is called in writeQueue; the completion of a object method is called in readQueue.
If the cache read/write is associated, we should call dispatch_async(dispatch_get_main_queue()) every the UI needs update.
Maybe put completion in main thread will be better?

for example:
change this

public func remove(key: String, completion: (() -> Void)? = nil) {
    dispatch_async(writeQueue) { [weak self] in
      guard let weakSelf = self else {
        completion?()
        return
      }

      weakSelf.cache.removeObjectForKey(key)
      completion?()
    }
  }

to

public func remove(key: String, completion: (() -> Void)? = nil) {
        dispatch_async(writeQueue) { [weak self] in
            guard let weakSelf = self else {
                dispatch_async(dispatch_get_main_queue()){
                    completion?()
                }
                return
            }

            weakSelf.cache.removeObjectForKey(key)
            dispatch_async(dispatch_get_main_queue()){
                completion?()
            }
        }
    }
@vadymmarkov
Copy link
Contributor

vadymmarkov commented May 23, 2016

@Hengyu It was intentional from the beginning, so that it's up to developer to dispatch in needed queue in the completion callback. But we could give another look at it and reconsider. What do you guys think @hyperoslo/ios

@zenangst
Copy link
Contributor

I think it would be hard to define a silver bullet for this, there might be some causes where you don't want it to go back to the main queue. I think it is better to let the developer decide which queue should be used and when, just like @vadymmarkov stated.

@dreymonde
Copy link

Hey! I’m just the user of Cache and want to share an opinion. In my point of view, the current approach is fine. Dispatching to the main queue «by default» is pretty bad move if you’re using NSOperations, for example. And there are a lot of other situations when you want to handle your object in the background, not interrupting main thread. So I think that writing dispatch_async(dispatch_get_main_queue()) is not that bad after all. 😄

@zenangst
Copy link
Contributor

I agree with @dreymonde, we will stick with the current approach for now.

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

No branches or pull requests

4 participants