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

Replace if let strongSelf = self and if let self = self checks in closures with modern if let self #19549

Closed
mokagio opened this issue Nov 1, 2022 · 3 comments · Fixed by #23836

Comments

@mokagio
Copy link
Contributor

mokagio commented Nov 1, 2022

There's nothing wrong with using if let strongSelf = self or guard let strongSelf = self. However, strongSelf makes the code a bit more cumbersome to read.

strongSelf or any other name different from self used to be required when we couldn't use that keyword in an assignment within a closure, but we now can and do so in various places, e.g.

We should remove the leftover instances of this old pattern and keep the codebase homogeneous.

Update, we can actually use an even better syntax:

if let self {
  ...
}

I renamed the issue accordingly to include acting on both strongSelf = self and self = self.

@twstokes
Copy link
Contributor

twstokes commented Nov 3, 2022

👋 @mokagio - how do we feel about SE-0345 shorthand in these cases? We may need to address it in the style guide either way.

@mokagio
Copy link
Contributor Author

mokagio commented Nov 7, 2022

I didn't know about SE-0345 or about the style guide.

Love them both. Thanks!

Yes. We should definitely adopt the new syntax because it's more compact and clear, as well as updating the style guide to mention it.

@mokagio mokagio changed the title Replace if let strongSelf = self checks in closures with modern if let self = self Replace if let strongSelf = self and if let self = self checks in closures with modern if let self Nov 7, 2022
@mokagio
Copy link
Contributor Author

mokagio commented Jan 30, 2023

I just noticed that a recent SwiftLint version add a rule for this: realm/SwiftLint#4202 🦾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants