Skip to content

Instantly share code, notes, and snippets.

@nicklockwood nicklockwood/gist:7559729

Last active Apr 27, 2016
Embed
What would you like to do?
A proposal for an alternative NSNotificationCenter API that doesn't suck

Method

- (void)addObserver:(id)observer
            forName:(NSString *)name
             object:(id)object
              queue:(NSOperationQueue *)queue
         usingBlock:(void (^)(NSNotification *note, __weak id observer))block;

Usage

[[NSNotificationCenter defaultCenter] addObserver:self
                                          forName:NSSomeNotificationName
                                           object:nil
                                            queue:[NSOperationQueue mainQueue]
                                       usingBlock:^(NSNotification *note, __weak id observer) {
                                                      NSLog(@"self: %@", observer); //Look Ma, no leaks!
                                                  }];

Discussion

The observer object takes the place of the token in the current block-based API and works like the observer in the selector-based API. The block is retained by NSNotificationCenter only for as long as the observer exists. NSNotificationCenter does not retain the observer, so if all other references are deleted it will be destroyed, along with the block.

For convenience, a weak reference to the observer is passed as a second parameter to the block. The block can use this to refer to the observer (which would be 'self' in typical usage) to avoid doing the weakify dance.

There is no need to call removeObserver: in the observer's dealloc method, as the observer reference is cleaned up automatically when the observer is released. To remove the observer before it is released, you can use the existing removeObserver: APIs.

@RuiAAPeres

This comment has been minimized.

Copy link

RuiAAPeres commented Nov 20, 2013

One small change, put the queue param, before the block one. Makes it more readable.

@jwilling

This comment has been minimized.

Copy link

jwilling commented Nov 20, 2013

This really is quite excellent and aligns with the thoughts I've been having on how to fix this API. It's tempting for me to write a wrapper that does exactly this, as long as someone doesn't beat me to it. ;)

@drewcrawford

This comment has been minimized.

Copy link

drewcrawford commented Nov 20, 2013

+1 from me on this.

There are still some issues, (like NSAssert, or some other macro expansion case) where you can inadvertantly create a retain cycle. If you are interested, I have some radars open on this (rdar://13126860: programmatically determine if a block captures self. rdar://9773229, using NSAssert inside a block causes the block to capture self) that seem worth duping. But I think those are the kind of issues that need to get fixed in Foundation or Clang and can't really be addressed third-party.

That said this is a concrete improvement that can be implemented at the third-party level, and I certainly think it's worth doing.

@nicklockwood

This comment has been minimized.

Copy link
Owner Author

nicklockwood commented Nov 20, 2013

@RuiAAPeres quite right - I wrote it based on a vague memory of the actual API. I've updated it to more closely match the existing API's naming and parameter order.

@nicklockwood

This comment has been minimized.

Copy link
Owner Author

nicklockwood commented Nov 20, 2013

@drewcrawford yes, the NSAssert thing is horrifying, although I typically disable asserts in production, so I don't think that's ever bitten me in practice.

I think there's a good argument for Apple simply making blocks-accessed variables weak by default, or at least making it a compiler error not to specify the retain status explicitly (which would force people to think about it).

Related: Is there any reason why all references to "self" in any context aren't weak by default? I can't think of any case where extending the life of a class via an inexplicit strong self reference (i.e. where you haven't assigned it to a strong property) isn't more likely to be a bug than not.

@nicklockwood

This comment has been minimized.

Copy link
Owner Author

nicklockwood commented Nov 20, 2013

@jwilling go for it. I'll race you ;-)

@drewcrawford

This comment has been minimized.

Copy link

drewcrawford commented Nov 20, 2013

@nicklockwood Joachim made a pretty good case against special-casing self in the blog comments.

Explicit storage duration is a new idea. But, this spreads the solution (variable declarations) potentially far away from the block. Of course, we could adopt the convention to always declare new variables right before a block declaration. That's something to think about.

Personally I think the solution is some way to express your intent that a block does not capture (self/whatever). And then, if clang finds itself doing that thing that you explicitly expected not to happen, you get a build error.

@nicklockwood

This comment has been minimized.

Copy link
Owner Author

nicklockwood commented Nov 20, 2013

@jwilling

This comment has been minimized.

@RuiAAPeres

This comment has been minimized.

Copy link

RuiAAPeres commented Nov 20, 2013

@jwilling one thing. The purpose of using your category, is to facilitate a rather tedious work (weak dance). By giving it a slightly "different" signature, it will only make people not use it.

If you insist with a different signature, it could be a better approach to leave the initial part of the method the same as the original one addObserver:.

@nicklockwood

This comment has been minimized.

Copy link
Owner Author

nicklockwood commented Nov 20, 2013

@jwilling yours suffers from the same design flaw as my (1.0) version did, that there is no way to explicitly deregister the notifications again except by allowing the observer to be released.

I was hoping there was a way to do it by having the observer object act as a proxy for the real observers (see version 1.0.1), but that ended up being way more complicated, and in any case didn't work properly as the associated objects weren't cleaned up correctly.

I eventually managed to solve this (in version 1.0.2) by swizzling removeObserver:name:object: to manually release the associated observers (tokens in your case) for the object.

@jwilling

This comment has been minimized.

Copy link

jwilling commented Nov 20, 2013

@RuiAAPeres: Amended, however I still do want to keep mine prefixed.

@nicklockwood: Fixed (partially).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.