26

I am subclassing NSOperation in Swift and need to override the isExecuting and isFinished properties since I am overriding the start method.

The problem I run into is how to preserve key-value observing (KVO) while also being able to override these properties.

Normally in Obj-C this would be rather easy to redeclare the properties as readwrite in the class extension JSONOperation () definition. However, I don't see this same capability in Swift.

Example:

class JSONOperation : NSOperation, NSURLConnectionDelegate
{
    var executing : Bool
    {
        get { return super.executing }
        set { super.executing } // ERROR: readonly in the superclass
    }

    // Starts the asynchronous NSURLConnection on the main thread
    override func start()
    {
        self.willChangeValueForKey("isExecuting")
        self.executing = true
        self.didChangeValueForKey("isExecuting")

        NSOperationQueue.mainQueue().addOperationWithBlock(
        {
            self.connection = NSURLConnection(request: self.request, delegate: self, startImmediately: true)
        })
    }
}

So here is the solution I have come up with, but it feels awfully ugly and hacky:

var state = Operation()

struct Operation
{
    var executing = false
    var finished = false
}

override var executing : Bool
{
    get { return state.executing }
    set { state.executing = newValue }
}

override var finished : Bool
{
    get { return state.finished }
    set { state.finished = newValue }
}

Please tell me there is a better way. I know I could make a var isExecuting instead of the whole struct, but then I have two similarly named properties which introduces ambiguity and also makes it publicly writable (which I do not want).

Oh what I would do for some access modifier keywords...

Erik
  • 12,730
  • 5
  • 36
  • 42
  • 2
    BTW, the `start` function should also be checking `if (cancelled) {...}`, and if so, immediately `finish` the operation and quit. – Rob Sep 21 '14 at 16:53

3 Answers3

30

As David said, you can implement both a getter and setter in the subclass property override.

But, when defining asynchronous/concurrent operations (i.e. those operations that will complete asynchronously), it is critical to call the will/didChangeValueForKey for isFinished and isExecuting. If you don't, operations won't be released, dependencies won't be honored, you'll have problems is maxConcurrentOperationCount, etc.).

So I would therefore suggest:

private var _executing: Bool = false
override var executing: Bool {
    get {
        return _executing
    }
    set {
        if _executing != newValue {
            willChangeValueForKey("isExecuting")
            _executing = newValue
            didChangeValueForKey("isExecuting")
        }
    }
}

private var _finished: Bool = false;
override var finished: Bool {
    get {
        return _finished
    }
    set {
        if _finished != newValue {
            willChangeValueForKey("isFinished")
            _finished = newValue
            didChangeValueForKey("isFinished")
        }
    }
}

By the way, checking to see if _executing and _finished have changed is not critical, but it can sometimes be useful when writing custom cancel methods or the like.


Update:

More than once, people have pointed to the new finished/executing properties in NSOperation.h and concluded that the appropriate KVO keys would be finished/executing. Generally, when writing KVO-compliant properties, that would be correct.

But NSOperationQueue does not observe the finished/executing keys. It observes the isFinished/isExecuting keys. If you don't perform the KVO calls for the isFinished/isExecuting keys, you can have problems (notably dependencies between asynchronous operations will fail). It's annoying, but that's how it works. The Configuring Operations for Concurrent Execution section of the Operation Queues chapter of the Concurrency Programming Guide is very clear on the topic of needing to perform the isFinished/isExecuting KVO calls.

While the Concurrency Programming Guide is dated, it's quite explicit regarding the isFinished/isExecuting KVO. And one can easily empirically validate that the guide still reflects the actual NSOperation implementation. By way of demonstration, see the unit tests in this Github demonstration of the appropriate KVO when using asynchronous/concurrent NSOperation subclass in NSOperationQueue.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • I didn't make the down vote, but you should take your advice and look at the headers. The property names are executing and finished. Apple frequently prepends "is" to accessor names where the property doesn't have them. – David Berry Sep 22 '14 at 15:03
  • 2
    Agreed. But that's not how `NSOperation`/`NSOperationQueue` is implemented. It's actual observing `isFinished`/`isExecuting`. It's non-standard; it's annoying; but it's how it works. I've updated my answer with clarification and link to demonstration of the appropriate keys (and illustrates problem if you don't post `isFinished`/`isExecuting`). – Rob Sep 22 '14 at 17:05
  • 1
    Interesting is that you can use `operation.valueForKey("executing")` and it works as expected. – David Berry Sep 22 '14 at 22:15
  • Definitely seems like a broken interface here. My guess is that it's because `NSOperation` predates properties. – David Berry Sep 22 '14 at 22:16
  • 1
    May also explain why a couple of years back, I couldn't get operationCount to work and wound up creating my own implementation... – David Berry Sep 22 '14 at 22:18
  • 2
    FYI: i converted and ran the ["Github demonstration of the appropriate KVO"](https://github.com/robertmryan/Operation-Test-Swift) with Xcode 7.1 iOS 9.1 Swift 2.1. The `isFinished`/`isExecuting` KVO keys PASS. The `finished`/`executing` keys FAIL. – marc-medley Oct 24 '15 at 02:41
  • 1
    I don't know when it happened but you can now use either "isFinished" or "finished" as the keypath for notification and operations will work correctly. – nevan king Aug 18 '18 at 11:47
  • @Rob take a look to https://github.com/robertmryan/Operation-Test-Swift/pull/3, thanks. – Lorenzo B Oct 14 '18 at 14:59
  • @nevanking I've noticed the same. Using `finished` or `isFinished` is the same. On the contrary, `invalidFinished` does not work as expected. – Lorenzo B Oct 14 '18 at 15:05
  • @LorenzoB I assume the change was for Swift. AFAIK it's using the "keypaths affecting" API. I don't know `invalidFinished` though. – nevan king Oct 14 '18 at 16:17
  • @nevanking `invalidFinished` is a key not valid. Creating a Unit Test, it fails. See my PR above. – Lorenzo B Oct 14 '18 at 18:24
  • @Rob have you had the chance to see my PR? Thanks, Lorenzo – Lorenzo B Oct 19 '18 at 07:31
  • Hi @Rob, what is the difference between your code here and the `AsynchronousOperation` code you've posted [here](https://stackoverflow.com/a/48104095/1077789) (another answer of yours)? – Isuru Apr 11 '20 at 04:13
  • 1
    @Isuru - They’re two ways of skinning the same cat. All that matters is that you do the appropriate KVO notifications for `isExecuting` and `isFinished`. Here, I’m manually doing the KVO notifications and there I linked them to the changing `state`. Both work fine. (I must say, though, that in the other question, I synchronize to ensure thread-safe access, something that I didn’t contemplate here.) – Rob Apr 11 '20 at 04:56
24

From the swift book:

You can present an inherited read-only property as a read-write property by providing both a getter and a setter in your subclass property override.

I think you'll find that this works:

override var executing : Bool {
    get { return _executing }
    set { 
        willChangeValueForKey("isExecuting")
        _executing = newValue 
        didChangeValueForKey("isExecuting")
    }
}
private var _executing : Bool
Rob
  • 415,655
  • 72
  • 787
  • 1,044
David Berry
  • 40,941
  • 12
  • 84
  • 95
  • Thanks! This solution is going to be as good as it can get for now. The `_executing` name implies it is private and should prevent most programmer error, even if it does leak the encapsulation. Without access modifiers for variables there's not much we can do about that yet. – Erik Jun 08 '14 at 19:16
  • Yep, one of the major drawbacks to implementation and interface being the same files, particularly with no access modifiers. – David Berry Jun 08 '14 at 19:18
  • @SiLo We now have the `private` access modifiers. – Rob Sep 21 '14 at 15:55
  • @David It actually is quite important to call `will`/`didChangeValueForKey` for the `isFinished` and `isExecuting` keys when implementing `asynchronous`/`concurrent` operations. – Rob Sep 21 '14 at 15:55
  • Hmm... that was months ago and I'm not sure why I added that comment. Something would've given it to me, but not sure what. Your observation certainly makes sense, so go with that :) – David Berry Sep 21 '14 at 18:29
  • 2
    @David that is closer, but `NSOperation` is observing `isFinished` and `isExecuting` notifications, not `finished` and `executing`. So, in your example, you have to do the KVN calls for `isExecuting` key name, rather than `executing`. – Rob Sep 22 '14 at 06:42
  • 1
    The property names are "finished" and "executing" The accessor names are "isFinished" and "isExecuting": property (readonly, getter=isExecuting) BOOL executing; property (readonly, getter=isFinished) BOOL finished; – David Berry Sep 22 '14 at 15:01
  • 1
    Agreed. But sadly `NSOperationQueue` is observing `isFinished`/`isExecuting`, even though that's not the standard KVO keys one would expect with properties called `finished`/`executing`. – Rob Sep 22 '14 at 17:07
  • Shouldn't there be some access synchronization? These properties in NSOperation are declared as atomic and can be accessed concurrently. Thread Sanitizer detects a data race with this implementation. – Vadim Yelagin Sep 26 '18 at 14:50
  • As long as the setter is only called from a single thread, there's no need for access synchronization. If there is other information that should also be updated *with* the executing property than access to both may require synchronization. Ie., if there's a result property you set at the same time as you set executing. Alternatively if you set the result *before* setting executing you should be fine. Synchronization may also be required for finished and executing combined. – David Berry Sep 26 '18 at 16:20
11

Swift 3.0 Answer Update:

private var _executing : Bool = false
override var isExecuting : Bool {
    get { return _executing }
    set {
        guard _executing != newValue else { return }
        willChangeValue(forKey: "isExecuting")
        _executing = newValue
        didChangeValue(forKey: "isExecuting")
    }
}


private var _finished : Bool = false
override var isFinished : Bool {
    get { return _finished }
    set {
        guard _finished != newValue else { return }
        willChangeValue(forKey: "isFinished")
        _finished = newValue
        didChangeValue(forKey: "isFinished")
    }
}
Mark
  • 16,906
  • 20
  • 84
  • 117
  • 4
    So ridiculous that Apple does provide this out of the box. We need to come up with our own custom solutions ... Why isn't there any ConcurrentOperation class to inherit from? – denis631 Mar 21 '18 at 09:58