NSNotificationCenter with blocks considered harmful

The only thing that surprised me was the reference cycle in NSAssert. Then again, given Apple's poor regard for TDD, that shouldn't surprise me.

    __weak MyClass* wself = self;
    self->observer = [[NSNotificationCenter defaultCenter]
                       addObserverForName:@"heyho"
                       object:nil queue:nil
                       usingBlock:^(NSNotification *n) {
                         MyClass *sself = wself;
                         if (sself) {
                           NSLog(@"%@", sself);
                         }
                       }];

Also, it's my understanding closures only cause retain cycles if the method taking the block copies the block, per the docs: "When a block is copied, it creates strong references to object variables used within the block." Therefore this dance is not necessary for e.g. [NSURLConnection sendAsynchronousRequest:queue:completionHandler:]. Is that correct?

If you over-do it then NSNotifications easily can become distributed goto. Once I had to add features to a Mac application that made heavy (ab)use of NSNotifications. Following code paths was a nightmare.

I tend to prefer the delegate pattern because there the relationships between objects are clear. Even if it means more work (creating glue code) - your sanity is worth it.

http://stackoverflow.com/q/7764564/23643

Just a heads up though: The code snippet were hard to read because of the indentation. May I suggest

  cleanupObj = [[NSNotificationCenter defaultCenter]
              addObserverForName:notificationName
              object:nil
              queue:nil
              usingBlock:^(NSNotification *note) {
                // Code!
              }];

1. The test is depending on dealloc being done eagerly.

2. The Attempt class puts unsubscribe code in its dealloc method.

3. The Attempt class does not force callers to control its lifetime.

I'll go into more depth on these.

1. Cleanup is often deferred. Dealloc may only run AFTER the test instead of DURING the loop (e.g. I think NSArray defers releasing its items in some cases). Tests that depend on cleanup occurring eagerly, without explicitly waiting or forcing it, are brittle. Given that Attempt uses dealloc to unsubscribe, and we are testing that it unsubscribed, we should at the very least allocate it in an autoreleasepool that ends before we test that it unsubscribed.

2. Putting unsubscribe code in dealloc is a great way to upgrade minor memory leak bugs into major behavior bugs. The article gives fantastic examples of this happening many different ways. Don't rely on the object happening to go out of scope at the right time. If your object's lifetime is controlling whether or not side effects occur, that's a bad situation to be in.

3. Someone has to be responsible for when unsubscribing happens (since we took it away from dealloc), and the someone most in a position to know when is the caller. We should force them to tell us our subscription lifetime. I like to control lifetimes with cancellation tokens [1], so I'd write the attempt class like this:

    @implementation Attempt
    -(instancetype) initUntil:(TOCCancelToken*)untilCancelledToken {
        if (self = [super init]) {
            id cleanupObj = ... addObserverForName stuff from Attempt1 ...
            [untilCancelledToken whenCancelledDo:^{ 
                [NSNotificationCenter.defaultCenter removeObserver:cleanupObj];
            }];
        }
        return self;
    }
    - (void)setLocalCounter:(int)localCounter { ... }
    - (int)localCounter { ... }
    @end

    -(void)testExample {
        for(int i =0; i < 5; i++) {
            TOCCancelTokenSource* c = [TOCCancelTokenSource new];
            Attempt *a = [[Attempt alloc] initUntil:c.token];
            [NSNotificationCenter.defaultCenter postNotificationName:notificationName object:nil];
            [c cancel];
            
            XCTAssertEqual(counter, i+1, @"Unexpected value for counter.");
            XCTAssertEqual(1, attempt1.localCounter, @"Unexpected value for localCounter.");
        }
    }

1: http://twistedoakstudios.com/blog/Post7391_cancellation-toke...

self.block = ^{ typeof(self) strongSelf = weakSelf; if (strongSelf) { // ... } };

It lives only in the 'platform' layer of my code. Grudgingly because Apple enforce it.

It could be worse... they could have pulled a Google and used Java - then held back the tools they develop e.g. the Java VM because they are 'dangerous' and suggest that using native code is 'bad' if it is just for performance or cross-platform reasons.

At least the interoperability with sane programming languages in Objective-C is excellent.

I'd point out the vast majority of memory management issues I have is when using someone's refcounting or gc scheme. new and delete are exactly what i want all of the time. i like to tell the machine what to do and i have developed non-trivial software without leaks /before testing for leaks/ because once you have some practice with new and delete it becomes easy and you will never look back...

As a mechanism for broadcasting information throughout an app NSNotificationCenter is slower, more complicated and (apparently) more dangerous than my hand rolled code. That should be shocking and its counter to the common idea that 3rd party and especially OS libraries should be better... there are a number of cases where they measurably aren't.

(The overhead of the objective C message mechanism - or even the RTTI mechanism which is a small part of that, exceeds that of adding or reading something from a well implemented threadsafe data structure (which is the first step to either of these things in most cases))