Clarity and Optional mapping: looking for opinions

A discussion on Swift-Evolution recently verged onto how you cleanly look-up an optional key in a dictionary. That is:

let dict: [SomeType: ...] = ...
let key: SomeType? = someCall()
let value = dictionary[key] but only if key is non-nil

Dave Abrahams pointed out the standard syntax for this query is as follows:

if let value = key.map({ dict[$0] }) {...}
if let value = key.flatMap({dict[$0]}) {...} // thanks @oisdk
// This gist points out the issue with map vs flatMap

This approach uses optional mapping , returning and optionally-binding a value if the key is .Some(...) rather than .None. It’s simple and uses a single binding statement, along with the mapped look-up.

While I applaud parsimony, I can’t help but feel that the following (more verbose) multi-step version is easier to follow with a minimal loss in efficiency:

if let key = key, value = dict[key] {...}

This compound if-let statement optionally-binds the key and then performs the lookup, with a second optional binding. It’s not significantly different in approach from the optional map but it tells the story better. At least to me.

The downside, as Lily Ballard points out,  is that it binds an extra variable. In this case, it’s no big deal as key was already defined as part of the problem statement. But what if you rephrase the call as such:

if let key = foo(), value = dict[key] { ... }

This could potentially shadow an existing variable versus the previous case which does so intentionally. If so, this could cause conflict in the statements in success clause. It’s not a likely scenario but it’s one that should be considered in contrast to the Optional-mapping, which has no such issues:

if let value = foo().map({dict[$0]}) { ... }
if let value = foo().flatMap({dict[$0]}) { ... } // thanks again @oisdk

Even so, I still think the two-stage approach wins in clarity. The functional chaining has become more complex and in real world situations would likely use longer, more-involved symbols. The code is more fluent but also harder to track.

For me, it breaks down into the following questions:

  • Does the Optional map approach produce more efficient code? Slightly
  • Does the Optional map approach more effectively avoid potential errors? Possibly, with unplanned shadowing
  • Does the Optional map approach better communicate the programming story? Not for me, but it may for people more comfortable with thinking “mapping” and “Optional” together.
  • Will the Optional map approach be more maintainable? I don’t think so.

Those are my thoughts. Do you have strong opinions one way or the other?

7 Comments

  • None of this is great; dict[key?] would be great.

    • extension Dictionary {
         subscript(key: Key?) -> Dictionary.Value? {
            guard let key = key else {return nil}
            return self[key]
         }
      }
      
      let value = dict[someCall()]
  • I really prefer the “if let key = key, value = dict[key]”. Just because my fellow developers would immediately see what’s going on.

  • Here’s the kind of thing I personally like to use:

        let value = getKey().flatMap(dict.lookup)

    Here `lookup()` is just another name for subscripting, but as a regular method instead. (You could alternatively use the Cocoa-style name `valueForKey()`.)

    However such abstract names as `dict` and `getKey()` don’t lend themselves to understandability, especially alongside another generic name like `flatMap()`. But I think it’s redeemed when you’ve got well-named variables, e.g.:

        if let profile = loadProfile()?.flatMap(profileCache.lookup) {
          // we have a cached profile
        }

    (The specific example isn’t great, but the point is to hopefully demonstrate how domain-specific names improve this code over generic names.)

  • I really do prefer for clarity your version unwrapping the key first. It is far clearer what you are doing.

    As has been said, it would be slightly less efficient but perhaps this might be something the compiler can note, that you never use the key afterwards, so optimise it out?

  • The compound if let statements are much clearer, so I’d prefer that.

  • The functional version would be much nicer if subscript were accessible by a name and we used a recognizable symbol for flatmap:

    if let value = foo() => dict.subscript

    Otherwise “if let” wins at readability for me. One could also use a temporary symbol if avoiding shadowing is important:

    if let $ = foo(), value = dict[$] { … }