2

I'm attempting to increase legibility of my KVO observeValueForKeyPath implementation by replacing the typical long string of nested if/else statements with a single switch statement.

So far, the only thing that's actually worked is:

private let application = UIApplication.sharedApplication()

    switch (object!, keyPath!) {

    case let (object, "delegate") where object as? UIApplication === application:
        appDelegate = application.delegate
        break

    ...

    default:
        super.observeValueForKeyPath(keyPath, ofObject: object, change: change, context: context)
    }

Which, if anything, is even harder to read than:

    if object as? UIApplication === application && keyPath! == "delegate" {

    }
    else {

    }

Does anybody have a good model for using switch in observeValueForKeyPath (and similar methods)

EDIT: Relevant to @critik's question below, here's more of the code to demonstrate the problems with just using switch (object as! NSObject, keyPath!) {:

private let application = UIApplication.sharedApplication()
private var appDelegate : UIApplicationDelegate?
private var rootWindow : UIWindow?

public override func observeValueForKeyPath(
    keyPath: String?,
    ofObject object: AnyObject?,
    change: [String : AnyObject]?,
    context: UnsafeMutablePointer<Void>) {

    switch (object as! NSObject, keyPath!) {
    case (application, "delegate"):
        appDelegate = application.delegate
        (appDelegate as? NSObject)?.addObserver(self, forKeyPath: "window", options: [.Initial], context: nil)
        break

    case (appDelegate, "window"):
        rootWindow = appDelegate?.window?.flatMap { $0 }
        break

    case (rootWindow, "rootViewController"):
        rebuildViewControllerList(rootWindow?.rootViewController)
        break

    default:
        super.observeValueForKeyPath(keyPath, ofObject: object, change: change, context: context)
    }
}            
David Berry
  • 40,941
  • 12
  • 84
  • 95
  • All you're really saying is that the whole KVO architecture sucks. That's true but it's not new news. We've been up and down this road many, many times. There are many obvious workarounds but in the end it's hard to avoid a huge bottleneck, where everything passes thru this one method, and a massive switch is the standard way of dealing with it. – matt Jan 20 '16 at 16:36
  • @matt more annoying to me right now is how incompletely implemented, or perhaps incompletely used) it is on iOS. There are many properties on system objects which can't be usefully observed, `UIViewController.presentedViewController` for instance. – David Berry Jan 20 '16 at 16:39
  • Be thankful. Do you know how KVO works? It reaches right into your object and swizzles it, substituting another object for your object! It's the Work Of The Devil. The fact that we sometimes _have_ to use it is in fact the problem. – matt Jan 20 '16 at 16:43
  • Yeah, I've been using KVO since pretty much the dawn of time. Conceptually it's a very powerful mechanism, particularly so on MacOS, where you can bind views directly to properties. I've often lamented the ability to do that on iOS, to the extent that I've hacked together solutions that let me do that on at least a couple of occasions :) – David Berry Jan 20 '16 at 16:47
  • "where you can bind views directly to properties" Sure, I know. I have applications that rely heavily on this. But of course I can't figure out how they work. :) – matt Jan 20 '16 at 17:02
  • That and removing a non existent observer being a fatal error... – David Berry Jan 20 '16 at 17:09
  • And forgetting to remove observers in time... :) – matt Jan 20 '16 at 18:50
  • Yep, I want to scream "You're obviously checking for this, JUST FIX IT" – David Berry Jan 20 '16 at 19:00

2 Answers2

0

How about a switch on tuples:

switch (object as! NSObject, keyPath!) {

case (application, "delegate"):
    appDelegate = application.delegate

...

default:
    super.observeValueForKeyPath(keyPath, ofObject: object, change: change, context: context)
}

Note 1. Although I'm agains forced stuff in Swift (downcasts, unwraps, etc), you can safely force downcast to NSObject without getting a crash, as per this SO question, KVO is only available for NSObject subclasses.

Note 2. You also don't need a break in Swift, this will also shorten your code by at least one line :)

Community
  • 1
  • 1
Cristik
  • 30,989
  • 25
  • 91
  • 127
  • I tried a bunch of casting variants, but didn't try NSObject. Unfortunately, that doesn't seem to work if some of the targets are optional. "Expression pattern of type 'UIApplicationDelegate?' cannot match values of type 'NSObject'" – David Berry Jan 19 '16 at 18:09
  • @DavidBerry can you post the code that fails? Perhaps I'll find a solution for it :) – Cristik Jan 19 '16 at 18:13
  • Think I updated it with enough of the code to see the problems. – David Berry Jan 20 '16 at 00:19
  • @DavidBerry I updated my answer with a solution for the second `switch` – Cristik Jan 20 '16 at 07:57
  • This code compiles, but it doesn't do the same thing. Rather than checking to see if `object == application`, your code checks to see if `object is UIApplication` and then assigns it to a new variable named application, ie., it only does a type check. To see what I mean, change it to `case let (app as UIApplication, "delegate"):` and see that it still compiles. – David Berry Jan 20 '16 at 16:06
  • Hmm... you might be right, I did test only with one value, and this is why it worked for me. Will get back to you if I find a valid implementation. – Cristik Jan 20 '16 at 16:08
  • Don't worry about it over much unless you just like to solve problems :). I've taken a different approach which eliminates this chunk of code completely. – David Berry Jan 20 '16 at 16:10
  • I kinda like solving problems :). I reverted my answer meanwhile, to not lead other people into error. – Cristik Jan 20 '16 at 16:11
  • BTW, is your solution `switch` based of `if` based, or did you give up to `observeValueForKeyPath`? – Cristik Jan 20 '16 at 16:12
0

This doesn't really solve the problem of using switch in observeValueForKey but it does demonstrate how I simplified the problem space to eliminate the verbose code.

I created a utility class, KVOValueWatcher which allows me to add (and remove) KVO observation on a single property of an object:

public class KVOValueWatcher<ObjectType:NSObject, ValueType:NSObject> : NSObject {
    public typealias OnValueChanged = (ValueType?, [String:AnyObject]?) -> ()

    let object : ObjectType
    let keyPath : String
    let options : NSKeyValueObservingOptions
    let onValueChanged : OnValueChanged
    var engaged = false

    public init(object:ObjectType, keyPath:String, options : NSKeyValueObservingOptions = [], onValueChanged: OnValueChanged) {
        self.object = object
        self.keyPath = keyPath
        self.onValueChanged = onValueChanged
        self.options = options

        super.init()

        engage()
    }

    deinit {
        if(engaged) {
            print("KVOValueWatcher deleted without being disengaged")
            print("    object: \(object)")
            print("    keyPath: \(keyPath)")
        }

        disengage()
    }

    public func engage() {
        if !engaged {
            self.object.addObserver(self, forKeyPath: keyPath, options: options, context: nil)
            engaged = true
        }
    }

    public func disengage() {
        if engaged {
            self.object.removeObserver(self, forKeyPath: keyPath)
            engaged = false
        }
    }

    override public func observeValueForKeyPath(keyPath: String?, ofObject object: AnyObject?, change: [String : AnyObject]?, context: UnsafeMutablePointer<Void>) {
        self.onValueChanged(((object as? NSObject)?.valueForKeyPath(keyPath!) as? ValueType), change)
    }
}

My problem code then becomes:

    rootWindowWatcher = KVOValueWatcher(object: applicationDelegate as! NSObject, keyPath: "window", options: [.Initial]) { window, changes in
        self.rootViewWatcher?.disengage()
        self.rootViewWatcher = nil

        if let window = window {
            self.rootViewWatcher = KVOValueWatcher(object: window, keyPath: "rootViewController", options: [.Initial]) {
                [unowned self] rootViewController, changes in
                self.rootViewController = rootViewController
                self.rebuildActiveChildWatchers()
            }
        }
    }

The primary reason for the change was because it was becoming a nightmare to maintain all the different observations and get them properly added and removed. Adding the wrapper class eliminates the problem and groups watching a property and the action to be taken when the property changes in the same location.

David Berry
  • 40,941
  • 12
  • 84
  • 95