5

In WWDC 2019 video 721, the basis of the main example starts like this:

let trickNamePublisher = NotificationCenter.default.publisher(for: .newTrickDownloaded)
    .map { notification in
        return notification.userInfo?["data"] as! Data
    }

That seems inadvisable. What happens if there's no userInfo, or it contains no "data" key, or it isn't a Data? We'll force-unwrap nil and crash. What's the best practice here?

matt
  • 515,959
  • 87
  • 875
  • 1,141
  • 1
    In this particular example it is *your* code which posts the notification, therefore you know what is in the user data. A crash would reveal a programming error. – Martin R Jun 22 '19 at 17:02
  • 1
    I was about to say the same thing, but Martin beat me to it. The example shows a notification coming from a different part of the app; the fact that the data exists in there is an invariant. When this is not the case, then `compactMap` is preferred. – Itai Ferber Jun 22 '19 at 17:04
  • @ItaiFerber The point is that the video might have mentioned that. Besides, how can it _ever_ hurt to use `compactMap` and safe unwrapping instead? Hence the "best practice" in the question. – matt Jun 22 '19 at 17:41
  • 1
    @MartinR I'll add that distinction to the formulation of the answer. – matt Jun 22 '19 at 17:41
  • @matt It certainly doesn't hurt, ever - I think for this particular example, more folks are familiar with `map` over `compactMap`, and being able to gloss over the fact that `compactMap` is able to drop values from the stream (which starts raising more questions than are answered here) keeps the example succinct. I agree with you that `compactMap` should be used here. – Itai Ferber Jun 22 '19 at 19:25
  • @ItaiFerber If there's something else I need to know or say in my answer with regard to dropping values from the stream, do let me know what it is. – matt Jun 22 '19 at 20:16

1 Answers1

8

Use compactMap instead:

let trickNamePublisher = NotificationCenter.default.publisher(for: .newTrickDownloaded)
    .compactMap { $0.userInfo?["data"] as? Data }

If our closure produces an Optional Data, it is unwrapped and we publish the Data. If our closure produces nil, nothing happens (nothing is published).

(It is surprising that the video doesn't write it that way. In Apple's defence, as MartinR and Itai Ferber point out, the video assumes that we ourselves are posting the notification, so we know for sure what's in the user info and force-unwrapping is reasonable. This question and answer focuses on the common situation where you've subscribed to a notification from a framework, such as Cocoa. Besides, I can't believe it's ever a bad idea to unwrap safely; in my own code, before the Combine framework, I have always unwrapped userInfo values safely even when I myself am posting the notification.)

matt
  • 515,959
  • 87
  • 875
  • 1,141