[swift-evolution] [Pitch] Changing NSObject dispatch behavior

[Kevin Ballard]

On Thu, Dec 15, 2016, at 02:33 PM, Kevin Ballard wrote:
> On Thu, Dec 15, 2016, at 02:29 PM, Charles Srstka wrote:
> > > On Dec 15, 2016, at 2:51 PM, Michael Ilseman via swift-evolution <swift-evolution at swift.org> wrote:
> > > 
> > >> I don't think we should ever make it possible to mark an entire class as `dynamic`. This just reintroduces the Obj-C problem, where many properties support KVO, but not all, and there's no indication on the property itself as to whether it supports it.
> > >> 
> > > 
> > > I’m not familiar enough with these kinds of bugs. Kevin, do you think the existing behavior aligns with or runs counter to safe-by-default?
> > 
> > The problem Kevin describes is still here; ‘dynamic’ itself is quite orthogonal to KVO as a concept. A method being declared ‘dynamic’ is no guarantee that it actually supports KVO, and likewise, a method does not need to be marked ‘dynamic’ in order to support KVO. You can, for example, either call willChangeValue() and didChangeValue() in your willSet and didSet accessors for a stored property, or for a computed property you can simply implement the proper accessor methods to describe the dependencies, as in the example below.
> > 
> > import Foundation
> > 
> > class Watcher: NSObject {
> > 	var kvoContext = 0
> > 	
> > 	init(watched: Watched) {
> > 		super.init()
> > 		watched.addObserver(self, forKeyPath: "foo", options: [], context: &kvoContext)
> > 	}
> > 	
> > 	override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
> > 		if context == &kvoContext {
> > 			print("foo changed; now it's \((object as! Watched).foo)")
> > 		} else {
> > 			super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
> > 		}
> > 	}
> > }
> > 
> > class Watched: NSObject {
> > 	// not a single ‘dynamic’ member in here:
> > 
> > 	class func keyPathsForValuesAffectingFoo() -> Set<String> { return ["bar"] }
> > 	var foo: Int {
> > 		get { return self.bar }
> > 		set(foo) { self.bar = foo }
> > 	}
> > 	
> > 	var bar: Int = 0 {
> > 		willSet { self.willChangeValue(forKey: "bar") }
> > 		didSet { self.didChangeValue(forKey: "bar") }
> > 	}
> > }
> > 
> > let watched = Watched()
> > let watcher = Watcher(watched: watched)
> > 
> > watched.bar = 5 // outputs: "foo changed; now it's 5”
> > 
> > - - - - - -
> > 
> > All ‘dynamic’ does for you vis a vis KVO conformance is to automatically add the willChangeValue() and didChangeValue() calls, *for a stored property.* For a computed property, that doesn’t help you, if the dependencies aren’t properly set up. For example, if we replace the Watched class with:
> > 
> > class Watched: NSObject {
> > 	dynamic var foo: Int {
> > 		get { return self.bar }
> > 		set(foo) { self.bar = foo }
> > 	}
> > 	
> > 	dynamic var bar: Int = 0 {
> > 		willSet { self.willChangeValue(forKey: "bar") }
> > 		didSet { self.didChangeValue(forKey: "bar") }
> > 	}
> > }
> > 
> > - - - - - -
> > 
> > and then change the “bar” property, no notifications will be fired, even though the value of ‘foo’ has indeed changed. So to a client of the class who can only see its interface and not the source code, ‘dynamic’ is useless on its own for determining KVO conformance.
> 
> The problem with that code isn't that `dynamic` doesn't work for computed properties. It does; if you mutate the `foo` property, you'll get the KVO notifications. The problem is you have one property that depends on another and you didn't set up the KVO machinery properly using automaticallyNotifiesObservers(forKey:) or automaticallyNotifiesObserversOf<key>() (incidentally in Swift you can write the latter as a `static let`, since that becomes a class method in Obj-C).

Oops, I mean keyPathsForValuesAffectingValue(forKey:) and keyPathsForValuesAffecting<Key>().

That said, your code as written actually sends 2 KVO change notices for "bar", and you do still need to implement automaticallyNotifiesObservers… to disable the automatic KVO notice for "bar".

-Kevin Ballard

> So yes, `dynamic` by itself doesn't mean that the property supports KVO. But there are very few reasons to use `dynamic` outside of supporting KVO, so it's a pretty good signal that the property does support it. And conversely, not having `dynamic` doesn't mean that it doesn't support KVO, though if it does have manual KVO support using will/didChangeValue(forKey:) then it should be documented as such.
> 
> > Bottom line: we really do need a new KVO-replacement system for Swift, one that could actually advertise itself as part of the interface instead of relying on documentation (especially since Apple isn’t interested in writing it anymore; the number of classes and methods that just say “No overview available.” in the documentation viewer in Sierra is really quite astounding). Unfortunately, this is probably additive, and thus probably won’t be heard until the next phase of swift-evolution.
> > 
> > Charles
> >