google / promises

Promises is a modern framework that provides a synchronization construct for Swift and Objective-C.
Apache License 2.0
3.8k stars 294 forks source link

retain cycle #125

Closed seanliu1 closed 4 years ago

seanliu1 commented 5 years ago
loadSomething().then { something in
  self.loadAnother(with: something)
}

func loadAnother(with something: Data) -> Promise<MyResult> {
  loadAnother().then { another in
    self.doSomething(with: something, and: another)
  }
}

This is piece of code from readme. Do we need to use [weak self]?

shoumikhin commented 4 years ago

Hi @seanliu1,

That code can be rewritten in the following way using GCD (let's ignore error handling), if that makes ownership clearer:


func loadSomething(_ completion: (Data) -> Void) {
 DispatchQueue.global().async {
    let something = MyLoader.syncLoadSomething()
    DispatchQueue.main.async {
      completion(something)
    }
  }
}

loadSomething { something in
  self.loadAnother(with: something) { result in
    // use result
  }
}

func loadAnother(with something: Data, _ completion: (MyResult) -> Void) {
  loadAnother { another in
    self.doSomething(with: something, and: another) { result in
      completion(result)
    }
  }
}

As you see, self gets captured by GCD at every completion handler. So the answer whether to make it weak is "maybe" :) It depends on whether you want self stay alive until all the async calls are completed, or you prefer to let it go earlier.

Thanks.

seanliu1 commented 4 years ago

Thanks for your detailed explanation. Yes, if we want weak self. What is the best way to do it. Do we create another error type for weak self, or we just return optional Promise. or we just fulfill with nil. Based on the best practice we should always fulfill or reject. If we use optional, when we chain them, it is not convenient at all. So I am curious what is the best practice for doing it?

shoumikhin commented 4 years ago

Again, it really depends.

If you capture a weak self, it's up to you how you're going to convey to the caller that self got deallocated. May throw an Error, or resolve with an empty result, or a Void, or just have an Optional result. I imagine the latter is probably the easiest in terms of code refactoring:

func loadAnother(with something: Data) -> Promise<MyResult?> {
  loadAnother().then { [weak self] another in
    self?.doSomething(with: something, and: another)
  }
}