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

Improve discussion and examples for rule to avoid unowned captures #240

Merged
merged 1 commit into from Oct 10, 2023

Conversation

calda
Copy link
Member

@calda calda commented Sep 25, 2023

Summary

This PR improves the discussion and examples for the "avoid unowned self" rule:

  • The examples now use unowned self and weak self, which feels less contrived than the previous examples
  • We now include discussion around explicitly capturing the instance members used in the closure (e.g. [planet] instead of [weak self] / self?.planet).

unowned captures are unsafe because they will cause the application to crash if the referenced object has been deallocated.

// WRONG: Crashes if `self` has been deallocated when closures are called.
final class SpaceshipNavigationService {
  let spaceship: Spaceship
  let planet: Planet
  
  func colonizePlanet() {
    spaceship.travel(to: planet, onArrival: { [unowned self] in
      planet.colonize()
    })
  }
  
  func exploreSystem() {
    spaceship.travel(to: planet, nextDestination: { [unowned self] in
      planet.moons?.first
    })
  }
}

weak captures are safer because they require the author to explicitly handle the case where the referenced object no longer exists.

// RIGHT: Uses a `weak self` capture and explicitly handles the case where `self` has been deallocated
final class SpaceshipNavigationService {
  let spaceship: Spaceship
  let planet: Planet
  
  func colonizePlanet() {
    spaceship.travel(to: planet, onArrival: { [weak self] in
      guard let self else { return }
      planet.colonize()
    })
  }
  
  func exploreSystem() {
    spaceship.travel(to: planet, nextDestination: { [weak self] in
      guard let self else { return nil }
      return planet.moons?.first
    })
  }
}

Alternatively, consider directly capturing the variables that are used in the closure. This lets you avoid having to handle the case where self is nil, since you don't even need to reference self:

// RIGHT: Explicitly captures `planet` instead of capturing `self`
final class SpaceshipNavigationService {
  let spaceship: Spaceship
  let planet: Planet
  
  func colonizePlanet() {
    spaceship.travel(to: planet, onArrival: { [planet] in
      planet.colonize()
    })
  }
  
  func exploreSystem() {
    spaceship.travel(to: planet, nextDestination: { [planet] in
      planet.moons?.first
    })
  }
}

@calda calda force-pushed the cal--improve-unowned-self-example branch from bf25ac1 to de0b16c Compare September 25, 2023 17:16
Copy link
Contributor

@jqsilver jqsilver left a comment

Choose a reason for hiding this comment

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

LGTM!

@calda calda merged commit a8283ed into master Oct 10, 2023
3 checks passed
@calda calda deleted the cal--improve-unowned-self-example branch October 10, 2023 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants