-
Notifications
You must be signed in to change notification settings - Fork 377
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
Remove shadowing of variables #61
Comments
These declarations are not repeats, although I agree the naming is confusing. The initial weakSelf is to avoid a retain cycle, but to actually use it safely in the block we need to make a strong reference first (otherwise it's possible for weakSelf to be deallocated mid-function from another thread.) The second weakSelf does exactly the same thing, except this time for the inner block, and then we need to make another strong reference for the same reason. In the example you highlight I actually skipped the final nil check since we're only calling one method and it's safe for strongSelf to be nil. The weak/strong dance is ugly but it's functionally necessary. If you have a suggestion to make it more concise I'd love to hear it! |
@jstn by default now, the warning for shadow declarations is enabled. While the code right now is functionally correct, it would be nice if it didn't use shadow declarations, if only to silence the warning. |
Interesting, I didn't know that. So it has to be even uglier : ( |
I am totally aware of the use of the weakSelf/strongSelf pattern. But I would change a few things: |
I would also recommend to use these macros to make things easier:
|
I disagree that it's unnecessary, but I've made my case, it's up to Mr Geddins now. The rest is just a matter of style. |
I don't know if weak references are needed at all in most of these cases. I'm pretty sure it's fine to refer to |
@irace is, of course, correct -- presuming the dispatch queue flushes. Retaining |
@segiddins This would be another argument (the first being the stylistic one) for not using weak references at all, correct? |
Yes. |
TMCache is completely full of repeated variable declarations with the same name. This should be removed.
The text was updated successfully, but these errors were encountered: