From f4c38805b24e02cd0adb08083988c118be3a2d6a Mon Sep 17 00:00:00 2001 From: Rob Amos Date: Wed, 21 Apr 2021 23:09:36 +1000 Subject: [PATCH 1/3] A `MutableFlagGroup` cannot function without its `Snapshot`, so it cannot be a weak reference. This is a leftover from the original plan where the tree was created with the Snapshot, but now `MutableFlagGroup`s are created lazily and not retained, so there is no retain cycle here. --- Sources/Vexil/Snapshots/MutableFlagGroup.swift | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Sources/Vexil/Snapshots/MutableFlagGroup.swift b/Sources/Vexil/Snapshots/MutableFlagGroup.swift index 0e2fe07f..c81a11b1 100644 --- a/Sources/Vexil/Snapshots/MutableFlagGroup.swift +++ b/Sources/Vexil/Snapshots/MutableFlagGroup.swift @@ -15,7 +15,7 @@ public class MutableFlagGroup where Group: FlagContainer, Root: Fla // MARK: - Properties private var group: Group - weak private var snapshot: Snapshot? + private var snapshot: Snapshot // MARK: - Dynamic Member Lookup @@ -45,15 +45,11 @@ public class MutableFlagGroup where Group: FlagContainer, Root: Fla /// public subscript (dynamicMember dynamicMember: KeyPath) -> Value where Value: FlagValue { get { - guard let snapshot = self.snapshot else { return self.group[keyPath: dynamicMember] } - - return snapshot.lock.withLock { + return self.snapshot.lock.withLock { self.group[keyPath: dynamicMember] } } set { - guard let snapshot = self.snapshot else { return } - // see Snapshot.swift for how terrible this is return snapshot.lock.withLock { _ = self.group[keyPath: dynamicMember] @@ -65,7 +61,7 @@ public class MutableFlagGroup where Group: FlagContainer, Root: Fla /// Internal initialiser used to create MutableFlagGroups for a given subgroup and snapshot /// - init (group: Group, snapshot: Snapshot?) { + init (group: Group, snapshot: Snapshot) { self.group = group self.snapshot = snapshot } From 6635e67d7558fcbafbc3810968c6d83a6954529d Mon Sep 17 00:00:00 2001 From: Rob Amos Date: Wed, 21 Apr 2021 23:35:36 +1000 Subject: [PATCH 2/3] The snapshot and group are never mutated so they can be `let`. --- Sources/Vexil/Snapshots/MutableFlagGroup.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Vexil/Snapshots/MutableFlagGroup.swift b/Sources/Vexil/Snapshots/MutableFlagGroup.swift index c81a11b1..c5a92072 100644 --- a/Sources/Vexil/Snapshots/MutableFlagGroup.swift +++ b/Sources/Vexil/Snapshots/MutableFlagGroup.swift @@ -14,8 +14,8 @@ public class MutableFlagGroup where Group: FlagContainer, Root: Fla // MARK: - Properties - private var group: Group - private var snapshot: Snapshot + private let group: Group + private let snapshot: Snapshot // MARK: - Dynamic Member Lookup From 849c9a01b16b80b85770a3647f1abf8fa195a9af Mon Sep 17 00:00:00 2001 From: Rob Amos Date: Wed, 21 Apr 2021 23:35:56 +1000 Subject: [PATCH 3/3] Moved the `CustomDebugStringConvertible` conformance to the extensions file with the others --- .../Vexil/Snapshots/Snapshot+Extensions.swift | 14 ++++++++++++++ Sources/Vexil/Snapshots/Snapshot.swift | 17 ----------------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/Sources/Vexil/Snapshots/Snapshot+Extensions.swift b/Sources/Vexil/Snapshots/Snapshot+Extensions.swift index 2ed326cc..c6289ef9 100644 --- a/Sources/Vexil/Snapshots/Snapshot+Extensions.swift +++ b/Sources/Vexil/Snapshots/Snapshot+Extensions.swift @@ -18,3 +18,17 @@ extension Snapshot: Hashable where RootGroup: Hashable { hasher.combine(self._rootGroup) } } + +extension Snapshot: CustomDebugStringConvertible { + public var debugDescription: String { + return "Snapshot<\(String(describing: RootGroup.self))>(" + + Mirror(reflecting: _rootGroup).children + .map { _, value -> String in + (value as? CustomDebugStringConvertible)?.debugDescription + ?? (value as? CustomStringConvertible)?.description + ?? String(describing: value) + } + .joined(separator: "; ") + + ")" + } +} diff --git a/Sources/Vexil/Snapshots/Snapshot.swift b/Sources/Vexil/Snapshots/Snapshot.swift index b95b5495..c37f7f29 100644 --- a/Sources/Vexil/Snapshots/Snapshot.swift +++ b/Sources/Vexil/Snapshots/Snapshot.swift @@ -200,23 +200,6 @@ public class Snapshot where RootGroup: FlagContainer { } -// MARK: - Debugging - -extension Snapshot: CustomDebugStringConvertible { - public var debugDescription: String { - return "Snapshot<\(String(describing: RootGroup.self))>(" - + Mirror(reflecting: _rootGroup).children - .map { _, value -> String in - (value as? CustomDebugStringConvertible)?.debugDescription - ?? (value as? CustomStringConvertible)?.description - ?? String(describing: value) - } - .joined(separator: "; ") - + ")" - } -} - - #if !os(Linux) typealias SnapshotValueChanged = PassthroughSubject