diff --git a/MMMArrayChanges.podspec b/MMMArrayChanges.podspec index df58b68..9262a2d 100644 --- a/MMMArrayChanges.podspec +++ b/MMMArrayChanges.podspec @@ -6,7 +6,7 @@ Pod::Spec.new do |s| s.name = "MMMArrayChanges" - s.version = "2.0.0" + s.version = "2.1.0" s.summary = "Helps finding (UITableView-compatible) differences between two arrays possibly of different types" s.description = s.summary s.homepage = "https://github.com/mediamonks/MMMArrayChanges" diff --git a/Sources/MMMArrayChanges/Array+diffUpdate.swift b/Sources/MMMArrayChanges/Array+diffUpdate.swift index a676255..a62ce66 100644 --- a/Sources/MMMArrayChanges/Array+diffUpdate.swift +++ b/Sources/MMMArrayChanges/Array+diffUpdate.swift @@ -55,10 +55,6 @@ extension Array { - Returns: `true`, if the array has changed, i.e. if elements were added or removed or `update` closure returned `true` for at least one element. - - - Complexity: - - Must be *O(n^2)* because removing elements from a dictionary is quoted at *O(n)*. */ @discardableResult public mutating func diffUpdate( @@ -70,7 +66,7 @@ extension Array { ) -> Bool { // We just use the compact logic here, since that will behave the same with a // non-optional transform closure. - return compactDiffUpdate( + compactDiffUpdate( elementId: elementId, sourceArray: sourceArray, sourceElementId: sourceElementId, @@ -79,7 +75,7 @@ extension Array { remove: remove ) } - + @discardableResult /// The same as ``diffUpdate(elementId:sourceArray:transform:update:remove)`` except that it /// behaves as a `Array.compactMap` instead of `Array.map`, for cases where your source array can contain @@ -92,58 +88,89 @@ extension Array { remove: ((_ element: Element) -> Void) ) -> Bool { - var changed = false + // We don't officially support duplicate IDs, but it's still useful to remove them from the source in the field. + func uniquedSourceArray() -> [SourceElement] { + var seen = Set() + return sourceArray.compactMap { sourceElement -> SourceElement? in + let id = sourceElementId(sourceElement) + if seen.contains(id) { + return nil + } else { + seen.insert(id) + return sourceElement + } + } + } + + // Special cases are fairly common and should be worth handling. + if self.isEmpty { + self = uniquedSourceArray().compactMap(transform) + return !self.isEmpty + } else if sourceArray.isEmpty { + self.forEach(remove) + self = [] + return true + } else if self.count == sourceArray.count && self.lazy.map(elementId) == sourceArray.lazy.compactMap(sourceElementId) { + var changed = false + for (old, new) in zip(self.lazy, sourceArray.lazy) { + if update(old, new) { + changed = true + } + } + return changed + } - // First building an index of the existing elements, i.e. ID -> Element. - var elementById = [ElementId: Element](uniqueKeysWithValues: self.map { (elementId($0), $0) }) + // Let's build an index of the existing elements, i.e. ID -> Element. + var elementById = [ElementId: Element?].init( + self.map { (elementId($0), $0) }, + // Again, we don't officially support non-unique IDs, but we'd rather not crash. + uniquingKeysWith: { first, _ in first } + ) + + var changed = false + let sourceArray = uniquedSourceArray() - let result = sourceArray.compactMap { (sourceElement) -> Element? in + let result = sourceArray.compactMap { sourceElement -> Element? in let id = sourceElementId(sourceElement) - if let element = elementById[id] { - // According to our index the current array already has a matching element, so just keep it... - elementById.removeValue(forKey: id) - // ...possibly updating. + switch elementById[id] { + case .some(let element?): + // The current array already has a matching element, so just keep it and update. if update(element, sourceElement) { // The update closure indicated that a change in the existing element should be counted // alongside with removals, additions and moves. changed = true } + // Mark as seen by replacing with a nil (instead of removing), so we can detect duplicates + // and avoid potential O(*n*) complexity of `removeValue`. + elementById[id] = .some(.none) return element - } else { - // There is no matching element in the current array, let's create it at - // this position as long as it transforms. - if let el = transform(sourceElement) { - changed = true - return el - } + case .some(.none): + // Duplicate ID, filtering out. return nil + case .none: + // There is no matching element, let's create it at this position as long as it transforms. + guard let el = transform(sourceElement) else { + return nil + } + changed = true + return el } } // Leftovers in the index mean removed elements and thus that the array had changes. - if !elementById.isEmpty { - changed = true - } + changed = changed || elementById.values.contains { $0 != nil } if !changed { - // Seems like no additions or removals, but it's possible that the order of elements has changed. + // No additions or removals, but it's possible that the order of elements has changed. assert(self.count == result.count) - for i in 0.. Cookie in - return Cookie(apiModel: plainCookie) + items = apiResponse.map { plainCookie in + Cookie(apiModel: plainCookie) } // ... notify observers about the whole list updated. @@ -157,29 +157,83 @@ class MMMArrayChangesTestCaseSwift : XCTestCase { XCTAssert(items[1] === animalCracker) } - private func diffUpdate(items: inout [Cookie], apiResponse: [CookieFromAPI]) -> Bool { + // We don't officially support duplicates, but it's better to not crash nor generate extra changes when we encounter them. + public func testDuplicatesInDiffUpdate() { + + var items: [Cookie] = [] + + let apiResponse: [CookieFromAPI] = [ + CookieFromAPI(id: 1, name: "Almond biscuit"), + CookieFromAPI(id: 2, name: "Animal cracker"), + CookieFromAPI(id: 2, name: "Oreo") + ] + XCTAssertTrue(self.diffUpdate(items: &items, apiResponse: apiResponse)) - return items.diffUpdate( + // Duplicates are removed for free. + XCTAssertEqual(items.map { $0.id }, ["1", "2"]) + + // Duplicate elements should not cause changes. + XCTAssertFalse(self.diffUpdate(items: &items, apiResponse: apiResponse)) + } + + public func testPerformance() { + + var items: [String] = [] + let all = (0..<100000).map { "\($0)" } + let firstHalf = Array(all.prefix(all.count / 1)) + let secondHalf = Array(all.suffix(all.count / 1)) + let random = all.shuffled() + + func update(_ source: [String], file: StaticString = #filePath, line: UInt = #line) { + items.compactDiffUpdate( + elementId: { $0 }, + sourceArray: source, + sourceElementId: { $0 }, + transform: { $0 }, + update: { old, new in false }, + remove: { old in + } + ) + XCTAssertEqual(items, source, file: file, line: line) + } + + measure { + for source in [ + all, // Just filling in, quite common case. + all, all, all,// Cases when nothing changes are also common and perhaps should have more weight. + firstHalf, // Removed one half. + secondHalf, // Replaced one half. + all, // Inserted one half. + random, // Changed order of items. + [] // Deleted all, another common case. + ] { + update(source) + } + } + } + + private func diffUpdate(items: inout [Cookie], apiResponse: [CookieFromAPI]) -> Bool { + items.diffUpdate( // We need to tell it how to match elements in the current and source arrays by providing IDs that can be compared. - elementId: { (cookie: Cookie) -> String in + elementId: { cookie in return cookie.id }, sourceArray: apiResponse, // We decided to use the same IDs that are used by the models, i.e. string ones. - sourceElementId: { plainCookie -> String in "\(plainCookie.id)" }, - transform: { (apiModel) -> Cookie in + sourceElementId: { plainCookie in "\(plainCookie.id)" }, + transform: { apiModel in // Called for every plain API object that has no corresponding "thick" cookie model yet, // i.e. for every new cookie. We create new "thick" models only for those. - return Cookie(apiModel: apiModel) + Cookie(apiModel: apiModel) }, - update: { (cookie, apiCookie) -> Bool in + update: { cookie, apiCookie -> Bool in // Called for every cookie model that still has a corresponding plain object in the API response. // Let's update the fields we are interested in and notify observers of every individual object. // Note that we could also return `false` here regardless of the change status of individual // elements, so the diffUpdate() call would only return true in case elements were added or removed. - return cookie.update(apiModel: apiCookie) + cookie.update(apiModel: apiCookie) }, - remove: { (cookie: Cookie) in + remove: { cookie in // Called for all cookies that don't have matching plain objects in the backend response. // Let's just mark them as removed just in case somebody holds a reference to them a bit longer than // needed and might appreciate knowing that the object they hold is not in the main list anymore.