apple / swift-algorithms

Commonly used sequence and collection algorithms for Swift
Apache License 2.0
5.9k stars 435 forks source link

Add a non-mutating lazy replaceSubrange #203

Open karwa opened 11 months ago

karwa commented 11 months ago

Sometimes, you want to insert/remove elements from a collection without creating a copy and mutating it.

It's kind of a generalisation of chain. And maybe joined. I still need to do the guide document and implement BidirectionalCollection, but I'm creating the PR now for discussion purposes. In particular:

Checklist

karwa commented 11 months ago

So, when checking this out using Godbolt, I find that the compiler produces shockingly good code. To the extent that, while I initially thought it might be a bit niche, I now wonder if it could be an interesting alternative to mutating operations for a lot of other applications - particularly SwiftUI applications.

Imagine I have some data, perhaps retrieved from the network or stored in some global datasource object. Let's say they're some kind of product, perhaps shoes, and I'm displaying them in a list/grid. I also have a boolean, telling me whether I should display a special call-to-action item in the list. This is a fairly standard thing to want.

The actual data that I want to use to drive my list/grid is composed from those two elements: the list of products and the boolean.

class ProductsListViewModel {
  var rawProductsList: [Product] = ...
  var showCallToAction: Bool = ...

  var viewItems: ??? // built by combining rawProductsList with showCallToAction
}

SwiftUI tends to emphasise computed properties for these kinds of things -- while I could make viewItems a stored Array-type property, I'd then need to make sure it's always up-to-date. Also, if the products list is a reference to non-owned data (such as a Binding), it may update even when this particular view isn't on-screen; in that case, we might be recomputing the viewItems unnecessarily.

Computed properties, on the other hand, are always up-to-date, taking account of the latest list of products and the latest state of the boolean. With @Observable, these kinds of data dependencies will even be tracked automatically.

However, within the implementation of the computed property, we must make a copy of the products list in order to inject elements in to it. The lack of lazy insertions also means we have to perform eager transformations such as map.

class ProductsListViewModel {
  var rawProductsList: [Product] = ...
  var showCallToAction: Bool = ...

  var viewItems: some Collection<ListItem> {
    var result = rawProductsList.map { .product($0) }
    if showCallToAction {
      result.insert(.callToAction, at: min(result.count, 4))
    }
    return result
  }
}

This is unfortunate, since viewItems will likely be called in a SwiftUI body property, meaning the map copy, and potential second copy and element shuffling caused by the insert, will happen in this performance-critical function.

With lazy insertions, we can avoid that:

class ProductsListViewModel {
  var rawProductsList: [Product] = ...
  var showCallToAction: Bool = ...

  var viewItems: some Collection<ListItem> {
    let result = rawProductsList.lazy.map { .product($0) }
    let ctaPosition = result.index(result.startIndex, offsetBy: 4, limitedBy: result.endIndex) ?? result.endIndex
    if showCallToAction {
      return result.replacingSubrange(ctaPosition..<ctaPosition, with: CollectionOfOne(.callToAction))
    } else {
      return result.replacingSubrange(result.startIndex..<result.startIndex, with: EmptyCollection()) // no-op.
    }
  }
}

viewItems has the same contents as before, but now we're able to use a lazy map transformation on the products list, and our insertion of the call-to-action prompt also won't copy or shuffle elements around. I'd expect it to be cheaper to call this from within a SwiftUI body function.

Iterating such a collection is more expensive than iterating an array, but because the compiler does such a good job with the generated code, there's a very good chance that it's still a net positive.

Unfortunately, it is also more awkward to use. Which is why another idea that I have WRT naming is to create a .overlay namespace struct (similar to .lazy), on which we can implement similar convenience methods to RangeReplaceableCollection. So people could write things like:

result.overlay.insert(.callToAction, at: ctaPosition)
// instead of
result.replacingSubrange(ctaPosition..<ctaPosition, with: CollectionOfOne(.callToAction))

(Array obviously has integer indexing, which is supremely convenient. We can't quite match that, but we could perhaps add some convenience APIs such as insert(_: Element, atOffset: Int))

karwa commented 11 months ago

One issue is that, if you're only conditionally applying a replacement while returning an opaque type, you need to perform a no-op replacement which ensures both branches have the same type. e.g:

if showCallToAction {
  return result.overlay.inserting(.callToAction, at: min(4, result.count))
} else {
  // Must return an OverlayCollection<Base, CollectionOfOne<ListItem>> which only contains the base elements.
  // That's not trivial.
}

What we can do instead is to (internally) make the overlay elements optional (i.e. the stored property overlay would become an Optional<Overlay>). That means we could always construct a no-op overlay wrapper with any overlay type.

To expose that feature, we could add a conditional overlay function:

extension Collection {

  @inlinable
  public func overlay<Overlay>(
    if condition: Bool,
    _ makeOverlay: (OverlayCollectionNamespace<Self>) -> OverlayCollection<Self, Overlay>
  ) -> OverlayCollection<Self, Overlay> {
    if condition {
      return makeOverlay(self.overlay)
    } else {
      return OverlayCollection(base: self, overlay: nil, replacedRange: startIndex..<startIndex)
    }
  }
}

Allowing usage like:

var viewItems: some Collection<ListItem> {
  let result = rawProductsList.lazy.map { .product($0) }
  return result.overlay(if: showCallToAction) {
    $0.inserting(.callToAction, at: min(4, $0.count))
  }
}
lgreydev commented 10 months ago

replaceSubrange - Nice!

CTMacUser commented 10 months ago

For the single-element removal method, isn't "removingSubrange(position..<position)" wrong? The range above specifies zero elements. You'll need "position ..< NEXT(position)" instead, where NEXT is replaced with the appropriate code.

karwa commented 10 months ago

@CTMacUser Yes, good catch!

CTMacUser commented 10 months ago

OverlayCollection.makeIndex(_:) can't work if the source collections use the same Index type, because then the two overloads would have identical signatures. Possible fix: explicitly use the words "base" or "overlay" within the methods' names.

CTMacUser commented 10 months ago

You could add an optimized .isEmpty

  @inlinable
  public var isEmpty: Bool {
    base[..<replacedRange.lowerBound].isEmpty && base[replacedRange.upperBound...].isEmpty && (overlay?.isEmpty ?? true)
  }
karwa commented 10 months ago

OverlayCollection.makeIndex(_:) can't work if the source collections use the same Index type, because then the two overloads would have identical signatures. Possible fix: explicitly use the words "base" or "overlay" within the methods' names.

Yes it can, because whether we're constructing a base or overlay index is determined based on unbound generics. Specialisation happens later, and does not affect overload resolution. This is one of the ways in which Swift generics are not like C++ templates.

For example, the very first test appends an Array<Int> to a Range<Int>. Both types use Int as their index.

karwa commented 10 months ago

@natecook1000 Thoughts?

CTMacUser commented 10 months ago

In my run through of 598946f, that last default of true in isEmpty was never triggered. That's because the trigger would be excluding an overlay on an already-empty collection. My quick test case, appended to testConditionalReplacement:

    func getNoNumbers(shouldInsert: Bool) -> OverlayCollection<Range<Int>, CollectionOfOne<Int>> {
      (0..<0).overlay(if: shouldInsert) { $0.inserting(42, at: 0) }
    }

    do {
      let result = getNoNumbers(shouldInsert: true)
      XCTAssertEqualCollections(result, [42])
      IndexValidator().validate(result, expectedCount: 1)
    }

    do {
      let result = getNoNumbers(shouldInsert: false)
      XCTAssertEqualCollections(result, [])
      IndexValidator().validate(result, expectedCount: 0)
    }