-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Rule request: complete NSNotificationCenter detachment #1061
Comments
I don't understand this warning exactly. You write
IMHO if the dealloc of the super or the subclass is called it's pretty normal to remove the observers here, because the Object (even the sub or the super class) is gone from memory. It's enforcing the Observer to be in the notification center pointing on dead code. |
Imagine you have a View Controller. In This will unregister the view controller from ALL notifications, including the ones that the superclass registered. So if That's way this is an issue. The only place it's safe to do that is in |
Yeah but here it's the same principle. If my subclass is calling |
It's not, because Check the documentation: https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver?language=objc |
Okay now I get it. But the rule is wrong anyway. It's implemented for objective-c code and the right place to do this would be |
I will create a Pull Request to fix this issue. |
It's not implemented wrong, just the description is using Objective-C in this issue. Check the examples. |
Sorry it was my bad the code looked like this… deinit {
unregisterForNotification()
}
private func unregisterForNotification() {
NotificationCenter.default.removeObserver(self) // creates a warning of course
} Thanks for helping out @marcelofabri |
No problem! Unfortunately we can't (yet?) recognize this case, so this rule is pretty dumb in general (that's why it's opt-in). |
this behavior is absolutely correct. do not remove all observers. just remove the observers you are responsible for with open func removeObserver(_ observer: Any, name aName: NSNotification.Name?, object anObject: Any?) method. |
I know this is quite old though I'm getting warnings to add my remove observers method to |
From http://fauxpasapp.com/rules/#rule-CompleteNotificationCenterDetachment:
Warns when an object removes itself as an observer to all notifications (by invoking
-[NSNotificationCenter removeObserver:]
withself
).This can be a problem if a superclass or a subclass wants to keep observing some notifications. The recommended way to handle this is to remove self as an observer only for the specific notifications that you have previously begun observing.
This rule does not warn about such removals occuring in
-dealloc
.The text was updated successfully, but these errors were encountered: