0

So I'm using RxSwift and has a function that looks like this:

private func setAndVerifyTestmode(isOn: Bool) {
    parameterService.setTestMode(value: isOn)
      .flatMap { _ in self.parameterService.requestTestMode() }
      .subscribe( { [weak self] _ in
        //do stuff })
      .disposed(by: disposeBag)
}

I noticed that I had forgotten to use [weak self] in the .flatMap so I added it like this:

private func setAndVerifyTestmode(isOn: Bool) {
    parameterService.setTestMode(value: isOn)
      .flatMap { [weak self] (_: Int?) in 
          guard let self = self else { return .just(nil) }
          self.parameterService.requestTestMode() }
      .subscribe( { [weak self] _ in
        //do stuff })
      .disposed(by: disposeBag)
}

But then it gave me an error: Generic parameter Result could not be infered

I couldn't get around it so I tried using a nested function instead of the closure, ending up with this:

private func setAndVerifyTestMode(isOn: Bool) {
    func requestTestMode(_: Int?) -> Single<Int?> {
      parameterService.requestTestMode()
    }
     
parameterService.setTestMode(value: isOn)
      .flatMap(requestTestMode(_:))
      .subscribe( { [weak self] _ in
        //do stuff })
      .disposed(by: disposeBag)
}

Great, the compiler was happy and it works. And in my world this takes care of the memory leak issues since I'm no longer using a closure requiring a reference to self. But, a collegue of mine told me that this is the exact same thing as not using [weak self] in a closure; and that you are still subjected to memory leaks using a nested function. I can't really see that they are the same thing since there isn't even a reference to self anymore.

Anyway, the question I have is: Do I get around the issue with memory leaks, self and [weak self] using the nested function above, or is my collegue right: it's the same thing; there is no gain to what I did?

Joakim Sjöstedt
  • 824
  • 2
  • 9
  • 18
  • 1
    Sorry, I realize this is not what you asked, but let's go back one step. `weak self] (_: Int?) in self.parameterService.requestTestMode()` is indeed wrong. Why don't you just fix it? Change `self.parameterService` to `self?.parameterService`. I mean, the _whole point_ of `weak self` is that it turns `self` into an Optional so that it is not retained. To say `self` instead of `self?`, even if the compiler had not stopped you, would negate the entire purpose of the exercise: you are relating `self`, the very thing you said you didn't want to do. – matt Feb 24 '23 at 11:53
  • If there is no `self` captured, how is `parameterService.requestTestMode()` eventually getting called? – Sweeper Feb 24 '23 at 11:54
  • I do regard the entire premise of the question as mistaken, however. The need for this `weak self` dance is because otherwise a strong reference to self is stored by self, leading to a retain cycle where self is not released when it should be. To find out if that's happening, implement `deinit` and log it. If it's happening, fix it. If it's not, do nothing. But don't keep using `weak self` in this knee jerk fearful automatic way. Answer your own question: was there a memory issue in the first place? If so, and only if so, did your syntactic games solve it? Yes or no? – matt Feb 24 '23 at 12:04
  • relating > retaining – matt Feb 24 '23 at 12:05
  • Also what the heck is this mysterious `(_: Int?)` incantation that pervades your code? Just get rid of it, please. It does nothing and it annoys the compiler. – matt Feb 24 '23 at 12:25
  • @Sweeper It's getting called because `parameterService` is getting retained directly instead of indirectly through `self`. – Daniel T. Feb 24 '23 at 12:49
  • @DanielT. Oh wow the compiler is smart enough to do *that*? Would the behaviour change if it were `self.parameterService.requestTestMode()` instead? – Sweeper Feb 24 '23 at 12:52
  • I believe so, yes. – Daniel T. Feb 24 '23 at 12:55
  • Sorry, the above is wrong. In either case, it would only capture `parameterService`. – Daniel T. Feb 24 '23 at 13:01
  • @Sweeper My bad, I was wrong. The compiler is not smart enough to do that. I have updated my answer. – Daniel T. Feb 24 '23 at 14:00

1 Answers1

1

But, a collegue of mine told me that this is the exact same thing as not using [weak self] in a closure; and that you are still subjected to memory leaks using a nested function.

Your colleague is right. (This surprised me.)

Fortunately, there are easier ways:

The most obvious: .flatMap { [parameterService] _ in parameterService.requestTestMode() } By capturing parameterService directly instead of indirectly through self, you avoid capture of self.

Another option: .flatMap { [weak self] _ in self?.parameterService.requestTestMode() ?? .just(0) } The value to emit is problematic because you used a Single so you have to emit something. If you had used a Maybe or Observable, you could have just used .empty()

Like this: .asMaybe().flatMap { [weak self] _ in self?.parameterService.requestTestMode().asMaybe() ?? .empty() }

Lastly you could use a free function (one that is not bound to any class or struct) that is curried:

func requestTestMode(parameterService: ParameterService) -> (Int?) -> Single<Int?> {
    { _ in parameterService.requestTestMode() }
}

Which could be used like: .flatMap(requestTestMode(parameterService: parameterService)) And that is effectively the same as the first option I gave above.

Daniel T.
  • 32,821
  • 6
  • 50
  • 72