20

When working with a custom NSOperation subclass I noticed that the automatic key-value observing is disabled by the [NSOperation automaticallyNotifiesObserversForKey] class method (which returns NO at least for some key paths). Because of that the code inside of NSOperation subclasses is littered by manual calls to willChangeValueForKey: and didChange…, as visible in many code samples on the web.

Why does NSOperation do that? With automatic KVO support people could simply declare properties for the operation lifecycle flags (isExecuting etc.) and trigger the KVO events through the accessors, ie. the following code:

[self willChangeValueForKey:@"isExecuting"];
executing = NO;
[self didChangeValueForKey:@"isExecuting"];
[self willChangeValueForKey:@"isFinished"];
finished = YES;
[self didChangeValueForKey:@"isFinished"];

…could be replaced by this:

[self setIsExecuting:NO];
[self setIsFinished:YES];

Is there a catch somewhere? I just overrode the automaticallyNotifiesObserversForKey to return YES and things seem to work fine.

zoul
  • 102,279
  • 44
  • 260
  • 354

4 Answers4

17

The most likely explanation is that the kvo keys don't match the standard conventions. Normally one has methods like -isExecuting and -setExecuting:, where the key path is @"executing". In the case of NSOperation, the key path is @"isExecuting" instead.

The other possibility is that most NSOperations don't actually have a method named -setIsExecuting: to change that value. Instead, they base the executing/finished flags on other internal state. In this case, one absolutely needs to use the explicit willChange/didChange notifications. For example, if I have an NSOperation that wraps an NSURLConnection, I may have 2 ivars, one named data that holds the downloaded data, and one named connection which holds the NSURLConnection, and I may implement the getters like so:

- (BOOL)isExecuting {
    return (connection != nil);
}

- (BOOL)isFinished {
    return (data != nil && connection == nil);
}

Now my -start method can use

[self willChangeValueForKey:@"isExecuting"];
data = [[NSMutableData alloc] init]; // doesn't affect executing, but is used later
connection = [[NSURLConnection connectionWithRequest:request delegate:self] retain];
[self didChangeValueForKey:@"isExecuting"];

to start executing, and

[self willChangeValueForKey:@"isExecuting"];
[self willChangeValueForKey:@"isFinished"];
[connection cancel];
[connection release];
connection = nil;
[self didChangeValueForKey:@"isFinished"];
[self didChangeValueForKey:@"isExecuting"];

to finish.

Lily Ballard
  • 182,031
  • 33
  • 381
  • 347
  • Kevin, i believe your example is inconsistent and should nil connection after cancel/release. Otherwise, isFinished won't return YES. – Steven Fisher May 02 '12 at 23:50
7

While I agree that overriding automaticallyNotifiesObserversForKey appears to work, but I personally forgo the isExecuting and isFinished properties altogether and instead define executing and finished properties, which, as Kevin suggests, is more consistent with modern conventions:

@property (nonatomic, readwrite, getter = isExecuting) BOOL executing;
@property (nonatomic, readwrite, getter = isFinished)  BOOL finished;

I then write custom setters for these two properties, which do the necessary isExecuting and isFinished notifications:

- (void)setExecuting:(BOOL)executing
{
    [self willChangeValueForKey:@"isExecuting"];
    _executing = executing;
    [self didChangeValueForKey:@"isExecuting"];
}

- (void)setFinished:(BOOL)finished
{
    [self willChangeValueForKey:@"isFinished"];
    _finished = finished;
    [self didChangeValueForKey:@"isFinished"];
}

This yields:

  • a more customary BOOL property declaration;
  • custom setters satisfy the strange notifications that NSOperation requires; and
  • I can now just use the executing and finished setters throughout my operation implementation, without littering my code with notifications.

I must confess that I like the elegance of overriding automaticallyNotifiesObserversForKey, but I just worry about unintended consequences.


Note, if doing this in iOS 8 or Yosemite, you will also have to explicitly synthesize these properties in your @implementation:

@synthesize finished  = _finished;
@synthesize executing = _executing;
Rob
  • 415,655
  • 72
  • 787
  • 1,044
0

I don't know why you are talking about NSOperation can not use automatic KVO. But I just try to verify that, therefore it can use KVO.

[self addObserver:self
       forKeyPath:@"isReady"
          options:NSKeyValueObservingOptionNew | NSKeyValueObservingOptionInitial
          context:&ctxKVO_CSDownloadOperation];

[self addObserver:self
       forKeyPath:@"isExecuting"
          options:NSKeyValueObservingOptionNew | NSKeyValueObservingOptionInitial
          context:&ctxKVO_CSDownloadOperation];

[self addObserver:self
       forKeyPath:@"isFinished"
          options:NSKeyValueObservingOptionNew | NSKeyValueObservingOptionInitial
          context:&ctxKVO_CSDownloadOperation];

[self addObserver:self
       forKeyPath:@"isCancelled"
          options:NSKeyValueObservingOptionNew | NSKeyValueObservingOptionInitial
          context:&ctxKVO_CSDownloadOperation];

...

- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary *)change context:(void *)context
{
    if (context == &ctxKVO_CSDownloadOperation) {
        NSLog(@"KVO: %@", keyPath);
    } else {
        [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
    }
}

The result:

2017-08-02 14:29:58.831 CSDownloader[77366:5089399] isReady : 1
2017-08-02 14:29:58.831 CSDownloader[77366:5089399] KVO: isReady
2017-08-02 14:29:58.831 CSDownloader[77366:5089399] isExecuting : 0
2017-08-02 14:29:58.831 CSDownloader[77366:5089399] KVO: isExecuting
2017-08-02 14:29:58.831 CSDownloader[77366:5089399] isFinished : 0
2017-08-02 14:29:58.832 CSDownloader[77366:5089399] KVO: isFinished
2017-08-02 14:29:58.832 CSDownloader[77366:5089399] isCancelled : 0
2017-08-02 14:29:58.832 CSDownloader[77366:5089399] KVO: isCancelled
2017-08-02 14:29:58.832 CSDownloader[77366:5089399] isReady : 1
2017-08-02 14:29:58.832 CSDownloader[77366:5089399] KVO: isReady
2017-08-02 14:29:58.833 CSDownloader[77366:5089399] isExecuting : 0
2017-08-02 14:29:58.833 CSDownloader[77366:5089399] KVO: isExecuting
2017-08-02 14:29:58.833 CSDownloader[77366:5089399] isFinished : 0
2017-08-02 14:29:58.833 CSDownloader[77366:5089399] KVO: isFinished
2017-08-02 14:29:58.833 CSDownloader[77366:5089399] isCancelled : 0
2017-08-02 14:29:58.833 CSDownloader[77366:5089399] KVO: isCancelled
2017-08-02 14:29:58.834 CSDownloader[77366:5089399] isReady : 1
2017-08-02 14:29:58.834 CSDownloader[77366:5089399] KVO: isReady
2017-08-02 14:29:58.834 CSDownloader[77366:5089399] isExecuting : 0

So I am really confused about this question and answers...

Chris Forever
  • 678
  • 1
  • 5
  • 18
-3

The NSOperationQueue is not observing isFinished or isExecuting, it is observing finished and executing.

isFinished is simply the the synthesized get accessor for the property finished. Automatic key-value observing notifications will be sent for this property unless your subclass has specifically opted out of automatic KVO notifications by implementing +automaticallyNotifiesObserversForKey or +automaticallyNotifiesObserversOf<Key> to return NO. If you have not opted out of automatic KVO notifications, you do not need to perform manual notifications using will/DidChangeValueForKey:. In your case, you were sending manual notifications for isFinished and isExecuting, which are not the key paths that NSOperationQueue observes.

TL;DR: These aren't the key paths that NSOperationQueue is looking for.

These are not the key paths you are looking for

executing and finished are the correct key paths, and they should be sending automatic KVO notifications.

If you are truly paranoid about KVO and want to send notifications for the get accessor key paths such as isFinished, register your property as a dependency of the key path:

+ (NSSet *) keyPathsForValuesAffectingIsFinished {
    NSSet   *result = [NSSet setWithObject:@"finished"];
    return result;
}
Community
  • 1
  • 1
quellish
  • 21,123
  • 4
  • 76
  • 83
  • 1
    You can't use your Jedi mind tricks on us. `NSOperation` and `NSOperationQueue` actually _do_ observe `isFinished`/`isExecuting` (as correctly described in the documentation). [This is easily demonstrated.](https://github.com/robertmryan/Operation-Test-Objective-C) You appear to incorrectly infer from the `finished` and `executing` properties in `NSOperation.h` that `NSOperation`/`NSOperationQueue` internally must be observing `finished` and `executing` KVN. The `isFinished`/`isExecuting` KVN is not for the "paranoid", but rather is essential for anyone using use asynchronous operations. – Rob Sep 23 '14 at 15:24
  • Please refer to the documentation on compliance with key value coding and key value observing. The correct way to be key value observing compliant in this scenario is to register the property as a dependency of the getter key path. If you assert that KVO or NSOperation is broken or a "bad API", file a bug. – quellish Sep 23 '14 at 15:25
  • This is the third time you've asked me to file a bug because `NSOperation` behaves precisely as documented in the _Concurrency Programming Guide._ Lol. Sure, it's disappointing that Apple's long-standing KVN convention for `NSOperation` requires this non-standard manual change notification, and if it bothers you that it doesn't conform to traditional KVO-compliance, you should file a bug. But until Apple changes it, please stop advising people to not do the appropriate `isFinished`/`isExecuting` notifications for `NSOperation`, because it causes problems for concurrent/asynchronous operations. – Rob Sep 23 '14 at 15:42
  • "Sure, it's disappointing that Apple's long-standing KVN convention for NSOperation requires this non-standard manual change notification" This is absolutely not true, but if you have a problem with the documentation or the API, file a bug. If *you* feel that NSOperation is not KVO compliant - which is effectively what you are asserting - file a bug. – quellish Sep 23 '14 at 16:34
  • "stop advising people to not do the appropriate isFinished/isExecuting notifications". I am advising people to use KVO *correctly* and when overriding public readonly properties with writable properties, to *register the dependent key paths*, NOT send manual change notifications. Again, this is covered in the KVO documentation. – quellish Sep 23 '14 at 16:35
  • Good addition of the key path dependencies pattern to get the required `isFinished` (and other) notifications. – Rob Sep 23 '14 at 19:00