Peeling the Onion

Fewer Layers Means Fewer Points of Failure

I was refactoring some older Swift code, and had changed a Core Data model class to use Set<Content> rather than NSSet. The meant I got to take out a lot of as? statements scattered through the project checking the content of the set, and I found an interesting very well hidden logic error while making the changes.

As a simplified psuedo-code example:

func parseItems(setValue: NSSet, completion: CompletionHandler) {
  if let items: Array:[ItemType] = setValue as [ItemType] where !items.isEmpty {
    for item in items {
      guard let value = item.optionalValue else {
        return
      }
      let finalCompletion: CompletionHandler? = item === items?.last ? completion ? nil
      doWork(onItemID: item.itemID, withExtractedValue: value completion: finalCompletion)
    }
  }
  else {
    completion()
  }
}

The bug in the code is that if the last item's optionalValue is nil, the completion handler isn't going to be called. I have a much better long term fix in progress involving operation queues and dependent operations, but as a short term fix while getting the refactoring work done I ended up with:

func parseItems(items: Set<ItemType>, completion: CompletionHandler) {
  let itemDataParser = { (item: ItemType) -> (itemID: String, value: String)?
    guard let extractedValue = item.value else {
      return nil
    }
    return (itemID: item.itemID, value: extractedValue)
  }
  guard let itemData = items.flatMap(itemDataParser) where !itemData.isEmpty else {
    completion()
  }
  for (index, item) in itemData.enumerate() {
    let finalCompletion: CompletionHandler? = index == itemData.count.predecessor() ? completion : nil
    doWork(onItemID: item.itemID, withExtractedValue: item.value completion: finalCompletion)
  }
}

Using flatMap combines filtering the incoming data to only include items that can have the work done, and then I either call the completion immediately or I go on to actually process the data. Long term I'll be wrapping all of the work being done in operations, and then using operation dependancies to make the final completion wait until all of the operations are completed, but this avoid the potential problem that I spotted.