From 9f2197accb96484aa57df069061a32de01dd34be Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Fri, 4 Dec 2020 16:28:35 +0000 Subject: [PATCH 1/9] Cache `numberOfSection` --- .../Providers/ComposedSectionProvider.swift | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Sources/Composed/Providers/ComposedSectionProvider.swift b/Sources/Composed/Providers/ComposedSectionProvider.swift index 86cc3d6..0f086e2 100644 --- a/Sources/Composed/Providers/ComposedSectionProvider.swift +++ b/Sources/Composed/Providers/ComposedSectionProvider.swift @@ -55,14 +55,7 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd } } - public var numberOfSections: Int { - return children.reduce(into: 0, { result, kind in - switch kind { - case .section: result += 1 - case let .provider(provider): result += provider.numberOfSections - } - }) - } + public private(set) var numberOfSections: Int = 0 public init() { } @@ -165,6 +158,7 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd updateDelegate?.willBeginUpdating(self) children.insert(.section(child), at: index) + numberOfSections += 1 let sectionOffset = self.sectionOffset(for: child) updateDelegate?.provider(self, didInsertSections: [child], at: IndexSet(integer: sectionOffset)) updateDelegate?.didEndUpdating(self) @@ -181,6 +175,7 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd updateDelegate?.willBeginUpdating(self) children.insert(.provider(child), at: index) + numberOfSections += child.sections.count let firstIndex = sectionOffset(for: child) let endIndex = firstIndex + child.sections.count updateDelegate?.provider(self, didInsertSections: child.sections, at: IndexSet(integersIn: firstIndex.. Date: Fri, 4 Dec 2020 16:28:55 +0000 Subject: [PATCH 2/9] Add optimisation when appending a child --- .../Providers/ComposedSectionProvider.swift | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Sources/Composed/Providers/ComposedSectionProvider.swift b/Sources/Composed/Providers/ComposedSectionProvider.swift index 0f086e2..433637c 100644 --- a/Sources/Composed/Providers/ComposedSectionProvider.swift +++ b/Sources/Composed/Providers/ComposedSectionProvider.swift @@ -68,6 +68,12 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd public func sectionOffset(for provider: SectionProvider) -> Int { guard provider !== self else { return 0 } + switch children.last { + case .some(.provider(let lastProvider)) where lastProvider === provider: + return numberOfSections - provider.numberOfSections + default: + break + } var offset: Int = 0 @@ -94,6 +100,13 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd } public func sectionOffset(for section: Section) -> Int { + switch children.last { + case .some(.section(let lastSection)) where lastSection === section: + return numberOfSections - 1 + default: + break + } + var offset: Int = 0 for child in children { From 60be040e04d1393f48c2eabbd39a81e559ba38e4 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Mon, 14 Dec 2020 11:47:35 +0000 Subject: [PATCH 3/9] Cache `sections` --- .../Providers/ComposedSectionProvider.swift | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/Sources/Composed/Providers/ComposedSectionProvider.swift b/Sources/Composed/Providers/ComposedSectionProvider.swift index 433637c..5dad821 100644 --- a/Sources/Composed/Providers/ComposedSectionProvider.swift +++ b/Sources/Composed/Providers/ComposedSectionProvider.swift @@ -29,20 +29,10 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd open weak var updateDelegate: SectionProviderUpdateDelegate? - /// Represents all of the children this provider contains - private var children: [Child] = [] - /// Returns all the sections this provider contains - public var sections: [Section] { - return children.flatMap { kind -> [Section] in - switch kind { - case let .section(section): - return [section] - case let .provider(provider): - return provider.sections - } - } - } + public private(set) var sections: [Section] = [] + + public private(set) var numberOfSections: Int = 0 /// Returns all the providers this provider contains public var providers: [SectionProvider] { @@ -55,7 +45,8 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd } } - public private(set) var numberOfSections: Int = 0 + /// Represents all of the children this provider contains + private var children: [Child] = [] public init() { } @@ -173,6 +164,7 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd children.insert(.section(child), at: index) numberOfSections += 1 let sectionOffset = self.sectionOffset(for: child) + sections.insert(child, at: sectionOffset) updateDelegate?.provider(self, didInsertSections: [child], at: IndexSet(integer: sectionOffset)) updateDelegate?.didEndUpdating(self) } @@ -191,6 +183,7 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd numberOfSections += child.sections.count let firstIndex = sectionOffset(for: child) let endIndex = firstIndex + child.sections.count + sections.insert(contentsOf: child.sections, at: firstIndex) updateDelegate?.provider(self, didInsertSections: child.sections, at: IndexSet(integersIn: firstIndex.. Date: Mon, 14 Dec 2020 12:00:00 +0000 Subject: [PATCH 4/9] Add comments to offset append optimisation --- Sources/Composed/Providers/ComposedSectionProvider.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Sources/Composed/Providers/ComposedSectionProvider.swift b/Sources/Composed/Providers/ComposedSectionProvider.swift index 5dad821..4b912ca 100644 --- a/Sources/Composed/Providers/ComposedSectionProvider.swift +++ b/Sources/Composed/Providers/ComposedSectionProvider.swift @@ -59,6 +59,9 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd public func sectionOffset(for provider: SectionProvider) -> Int { guard provider !== self else { return 0 } + + // A quick test for if this is the last child is a small optimisation, mainly + // beneficial when the provider has just been appended. switch children.last { case .some(.provider(let lastProvider)) where lastProvider === provider: return numberOfSections - provider.numberOfSections @@ -91,6 +94,8 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd } public func sectionOffset(for section: Section) -> Int { + // A quick test for if this is the last child is a small optimisation, mainly + // beneficial when the section has just been appended. switch children.last { case .some(.section(let lastSection)) where lastSection === section: return numberOfSections - 1 From 73f63eb63e3f4c4aa61553eb13da242ec4fb9f6c Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Tue, 15 Dec 2020 12:04:13 +0000 Subject: [PATCH 5/9] Reset test values before each test --- .../ComposedSectionProvider.swift | 73 +++++++++++-------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/Tests/ComposedTests/ComposedSectionProvider.swift b/Tests/ComposedTests/ComposedSectionProvider.swift index 6a1699e..47a318a 100644 --- a/Tests/ComposedTests/ComposedSectionProvider.swift +++ b/Tests/ComposedTests/ComposedSectionProvider.swift @@ -8,35 +8,50 @@ final class ComposedSectionProvider_Spec: QuickSpec { override func spec() { describe("ComposedSectionProvider") { - let global = ComposedSectionProvider() - - let child1 = ComposedSectionProvider() - let child1a = ArraySection() - let child1b = ArraySection() - let child2 = ComposedSectionProvider() - let child2a = ComposedSectionProvider() - let child2b = ArraySection() - let child2c = ArraySection() - let child2z = ComposedSectionProvider() - let child2d = ArraySection() - let child2e = ComposedSectionProvider() - let child2f = ArraySection() - - child1.append(child1a) - child1.insert(child1b, after: child1a) - - child2.append(child2a) - child2a.append(child2c) - child2a.insert(child2b, before: child2c) - - child2.insert(child2z, after: child2a) - child2.append(child2d) - child2e.append(child2f) - child2.append(child2e) - global.append(child1) - global.append(child2) - - it("should contain 2 global sections") { + var global: ComposedSectionProvider! + var child1: ComposedSectionProvider! + var child1a: ArraySection! + var child1b: ArraySection! + var child2: ComposedSectionProvider! + var child2a: ComposedSectionProvider! + var child2b: ArraySection! + var child2c: ArraySection! + var child2z: ComposedSectionProvider! + var child2d: ArraySection! + var child2e: ComposedSectionProvider! + var child2f: ArraySection! + + beforeEach { + global = ComposedSectionProvider() + + child1 = ComposedSectionProvider() + child1a = ArraySection() + child1b = ArraySection() + child2 = ComposedSectionProvider() + child2a = ComposedSectionProvider() + child2b = ArraySection() + child2c = ArraySection() + child2z = ComposedSectionProvider() + child2d = ArraySection() + child2e = ComposedSectionProvider() + child2f = ArraySection() + + child1.append(child1a) + child1.insert(child1b, after: child1a) + + child2.append(child2a) + child2a.append(child2c) + child2a.insert(child2b, before: child2c) + + child2.insert(child2z, after: child2a) + child2.append(child2d) + child2e.append(child2f) + child2.append(child2e) + global.append(child1) + global.append(child2) + } + + it("should contain 6 global sections") { expect(global.numberOfSections) == 6 } From d99c672e1461d5c5c900f390b31996202402f327 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Tue, 15 Dec 2020 12:22:45 +0000 Subject: [PATCH 6/9] Enable code coverage for Composed scheme --- .swiftpm/xcode/xcshareddata/xcschemes/Composed.xcscheme | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.swiftpm/xcode/xcshareddata/xcschemes/Composed.xcscheme b/.swiftpm/xcode/xcshareddata/xcschemes/Composed.xcscheme index 964ec7d..1302a6d 100644 --- a/.swiftpm/xcode/xcshareddata/xcschemes/Composed.xcscheme +++ b/.swiftpm/xcode/xcshareddata/xcschemes/Composed.xcscheme @@ -26,7 +26,8 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" - shouldUseLaunchSchemeArgsEnv = "YES"> + shouldUseLaunchSchemeArgsEnv = "YES" + codeCoverageEnabled = "YES"> From 308b2e3f602631e7e6e7b91799817e5831b275c3 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Tue, 15 Dec 2020 12:24:55 +0000 Subject: [PATCH 7/9] Fix incorrect sections being removed --- Sources/Composed/Providers/ComposedSectionProvider.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Composed/Providers/ComposedSectionProvider.swift b/Sources/Composed/Providers/ComposedSectionProvider.swift index 4b912ca..a2e138c 100644 --- a/Sources/Composed/Providers/ComposedSectionProvider.swift +++ b/Sources/Composed/Providers/ComposedSectionProvider.swift @@ -265,7 +265,7 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd numberOfSections -= sections.count let sectionOffset = self.sectionOffset(for: provider) - indexes.map { $0 + sectionOffset }.forEach { self.sections.remove(at: $0) } + indexes.map { $0 + sectionOffset }.reversed().forEach { self.sections.remove(at: $0) } updateDelegate?.provider(provider, didRemoveSections: sections, at: indexes) } From 2ca262027c8f74f9dc2d554de2cb04561b19cdf7 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Tue, 15 Dec 2020 12:25:24 +0000 Subject: [PATCH 8/9] Add more `ComposedSectionProvider` tests --- .../ComposedSectionProvider.swift | 127 +++++++++++++++--- 1 file changed, 110 insertions(+), 17 deletions(-) diff --git a/Tests/ComposedTests/ComposedSectionProvider.swift b/Tests/ComposedTests/ComposedSectionProvider.swift index 47a318a..6189d45 100644 --- a/Tests/ComposedTests/ComposedSectionProvider.swift +++ b/Tests/ComposedTests/ComposedSectionProvider.swift @@ -16,10 +16,11 @@ final class ComposedSectionProvider_Spec: QuickSpec { var child2a: ComposedSectionProvider! var child2b: ArraySection! var child2c: ArraySection! - var child2z: ComposedSectionProvider! - var child2d: ArraySection! + var child2d: ArraySection! var child2e: ComposedSectionProvider! - var child2f: ArraySection! + var child2f: ArraySection! + var child2g: ComposedSectionProvider! + var child2h: ArraySection! beforeEach { global = ComposedSectionProvider() @@ -31,10 +32,11 @@ final class ComposedSectionProvider_Spec: QuickSpec { child2a = ComposedSectionProvider() child2b = ArraySection() child2c = ArraySection() - child2z = ComposedSectionProvider() - child2d = ArraySection() + child2d = ArraySection() child2e = ComposedSectionProvider() - child2f = ArraySection() + child2f = ArraySection() + child2g = ComposedSectionProvider() + child2h = ArraySection() child1.append(child1a) child1.insert(child1b, after: child1a) @@ -42,17 +44,18 @@ final class ComposedSectionProvider_Spec: QuickSpec { child2.append(child2a) child2a.append(child2c) child2a.insert(child2b, before: child2c) + child2a.insert(child2d, after: child2c) - child2.insert(child2z, after: child2a) - child2.append(child2d) - child2e.append(child2f) - child2.append(child2e) + child2.insert(child2e, after: child2a) + child2.append(child2f) + child2g.append(child2h) + child2.append(child2g) global.append(child1) global.append(child2) } - it("should contain 6 global sections") { - expect(global.numberOfSections) == 6 + it("should contain 7 global sections") { + expect(global.numberOfSections) == 7 } it("cache should contain 2 providers") { @@ -61,25 +64,37 @@ final class ComposedSectionProvider_Spec: QuickSpec { it("should return the right offsets") { expect(global.sectionOffset(for: child1)) == 0 + expect(global.sectionOffset(for: child1a)) == 0 + expect(global.sectionOffset(for: child1b)) == 1 expect(global.sectionOffset(for: child2)) == 2 expect(global.sectionOffset(for: child2a)) == 2 - expect(global.sectionOffset(for: child2z)) == 4 + expect(global.sectionOffset(for: child2b)) == 2 + expect(global.sectionOffset(for: child2c)) == 3 + expect(global.sectionOffset(for: child2d)) == 4 expect(global.sectionOffset(for: child2e)) == 5 + expect(global.sectionOffset(for: child2f)) == 5 + expect(global.sectionOffset(for: child2g)) == 6 + expect(global.sectionOffset(for: child2h)) == 6 expect(child2.sectionOffset(for: child2a)) == 0 - expect(child2.sectionOffset(for: child2z)) == 2 expect(child2.sectionOffset(for: child2e)) == 3 + expect(child2.sectionOffset(for: child2g)) == 4 + + expect(child2a.sectionOffset(for: child2b)) == 0 + expect(child2a.sectionOffset(for: child2c)) == 1 + expect(child2a.sectionOffset(for: child2d)) == 2 } context("when a section is inserted after a section provider with multiple sections") { var mockDelegate: MockSectionProviderUpdateDelegate! var countBefore: Int! + var newSection: ArraySection! beforeEach { mockDelegate = MockSectionProviderUpdateDelegate() global.updateDelegate = mockDelegate - let newSection = ArraySection() + newSection = ArraySection() countBefore = global.numberOfSections global.append(newSection) @@ -88,19 +103,38 @@ final class ComposedSectionProvider_Spec: QuickSpec { it("should pass the correct indexes to the delegate") { expect(mockDelegate.didInsertSectionsCalls.last!.2) == IndexSet(integer: countBefore) } + + it("should update the sections count") { + expect(global.numberOfSections) == 8 + } + + it("should contain the correct sections") { + expect(global.sections[0]) === child1a + expect(global.sections[1]) === child1b + expect(global.sections[2]) === child2b + expect(global.sections[3]) === child2c + expect(global.sections[4]) === child2d + expect(global.sections[5]) === child2f + expect(global.sections[6]) === child2h + expect(global.sections[7]) === newSection + } } context("when a section provider is inserted after a section provider with multiple sections") { var mockDelegate: MockSectionProviderUpdateDelegate! var countBefore: Int! var sectionProvider: ComposedSectionProvider! + var newSection1: ArraySection! + var newSection2: ArraySection! beforeEach { mockDelegate = MockSectionProviderUpdateDelegate() global.updateDelegate = mockDelegate sectionProvider = ComposedSectionProvider() - sectionProvider.append(ArraySection()) - sectionProvider.append(ArraySection()) + newSection1 = ArraySection() + newSection2 = ArraySection() + sectionProvider.append(newSection1) + sectionProvider.append(newSection2) countBefore = global.numberOfSections @@ -110,6 +144,22 @@ final class ComposedSectionProvider_Spec: QuickSpec { it("should pass the correct indexes to the delegate") { expect(mockDelegate.didInsertSectionsCalls.last!.2) == IndexSet(integersIn: countBefore..<(countBefore + sectionProvider.numberOfSections)) } + + it("should update the sections count") { + expect(global.numberOfSections) == 9 + } + + it("should contain the correct sections") { + expect(global.sections[0]) === child1a + expect(global.sections[1]) === child1b + expect(global.sections[2]) === child2b + expect(global.sections[3]) === child2c + expect(global.sections[4]) === child2d + expect(global.sections[5]) === child2f + expect(global.sections[6]) === child2h + expect(global.sections[7]) === newSection1 + expect(global.sections[8]) === newSection2 + } } context("when a section located after a section provider with multiple sections is removed") { @@ -131,6 +181,49 @@ final class ComposedSectionProvider_Spec: QuickSpec { it("should pass the correct indexes to the delegate") { expect(mockDelegate.didRemoveSectionsCalls.last!.2) == IndexSet(integer: countBefore - 1) } + + it("should contain the correct sections") { + expect(global.sections[0]) === child1a + expect(global.sections[1]) === child1b + expect(global.sections[2]) === child2b + expect(global.sections[3]) === child2c + expect(global.sections[4]) === child2d + expect(global.sections[5]) === child2f + expect(global.sections[6]) === child2h + } + } + + context("when multiple sections are removed") { + var mockDelegate: MockSectionProviderUpdateDelegate! + var countBefore: Int! + + beforeEach { + mockDelegate = MockSectionProviderUpdateDelegate() + global.updateDelegate = mockDelegate + + countBefore = global.numberOfSections + + child2.remove(child2a) + } + + it("should pass through the removed indexes to the delegate") { + expect(mockDelegate.didRemoveSectionsCalls.last!.2) == IndexSet([0, 1, 2]) + } + + it("should update the number of sections") { + expect(global.numberOfSections) == countBefore - 3 + } + + it("should pass itself to the delegate") { + expect(mockDelegate.didRemoveSectionsCalls.last!.0) === child2 + } + + it("should contain the correct sections") { + expect(global.sections[0]) === child1a + expect(global.sections[1]) === child1b + expect(global.sections[2]) === child2f + expect(global.sections[3]) === child2h + } } } } From 95ec49be70c4c3b03c6435c89ac9bc06eb7e14b3 Mon Sep 17 00:00:00 2001 From: Joseph Duffy Date: Wed, 16 Dec 2020 09:32:17 +0000 Subject: [PATCH 9/9] Update `numberOfSections` between delegate calls --- Sources/Composed/Providers/ComposedSectionProvider.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/Composed/Providers/ComposedSectionProvider.swift b/Sources/Composed/Providers/ComposedSectionProvider.swift index a2e138c..2b07882 100644 --- a/Sources/Composed/Providers/ComposedSectionProvider.swift +++ b/Sources/Composed/Providers/ComposedSectionProvider.swift @@ -222,11 +222,9 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd case let .section(child): sections = [child] sectionOffset = self.sectionOffset(for: child) - numberOfSections -= 1 case let .provider(child): child.updateDelegate = nil sectionOffset = self.sectionOffset(for: child) - numberOfSections -= child.sections.count sections = child.sections } @@ -235,6 +233,7 @@ open class ComposedSectionProvider: AggregateSectionProvider, SectionProviderUpd updateDelegate?.willBeginUpdating(self) children.remove(at: index) + numberOfSections -= sections.count self.sections.removeSubrange(firstIndex ..< endIndex) updateDelegate?.provider(self, didRemoveSections: sections, at: IndexSet(integersIn: firstIndex..