[ECO-5495] Directly use ObjectsPool inside LiveMap getter methods
#88
Conversation
WalkthroughUse a snapshot ObjectsPool via a new LiveMapObjectsPoolDelegate (nosync_objectsPool) and pass ObjectsPool directly into internal LiveMap helpers; update realtime conformance, public proxy types, and tests to the new delegate and replace per-id mock with an ObjectsPool-based mock. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Public as PublicDefaultLiveMap
participant Internal as InternalDefaultLiveMap
participant Delegate as LiveMapObjectsPoolDelegate
participant Pool as ObjectsPool
Client->>Public: get/size/entries/keys/values(...)
Public->>Delegate: read nosync_objectsPool
Delegate-->>Public: ObjectsPool (snapshot)
Public->>Internal: call internal API with ObjectsPool
Internal->>Pool: read entries / metadata
alt entry is tombstoned
Pool-->>Internal: tombstone metadata
Internal-->>Public: omit or convert accordingly
else not tombstoned
Internal-->>Public: convert values from pool
end
Public-->>Client: return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(none) Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
github-actions
bot
temporarily deployed
to
staging/pull/88/AblyLiveObjects
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swift (1)
59-63: Optional: make MapCreationArgs Sendable (if delegate is Sendable).If LiveMapObjectsPoolDelegate conforms to Sendable, consider this small improvement.
- internal struct MapCreationArgs { + internal struct MapCreationArgs: Sendable {Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift (1)
349-355: Nit: fix misleading comment about tombstones.The comment says “tombstonedAt is false” but the code sets a Date (meaning tombstoned). Tweak wording to avoid confusion.
- // tombstonedAt is false, so not considered tombstoned + // tombstonedAt is non-nil, so considered tombstonedTests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectsPoolDelegate.swift (3)
18-27: Add convenience init to pre-seed entries.Makes tests shorter and avoids a separate write to objects.
Apply:
extension MockLiveMapObjectsPoolDelegate { + convenience init(internalQueue: DispatchQueue, objects: [String: ObjectsPool.Entry]) { + self.init(internalQueue: internalQueue) + self.objects = objects + } }
29-36: Setter recreates the pool; document identity reset or offer a merge path.Rebuilding the pool invalidates previously obtained Entry/LiveObject references. Fine for most tests, but please document it or add a merge helper if tests need to preserve identities.
38-40: Clarify thread-safety semantics of nosync_objectsPool.Suggest a short doc: "Returns a snapshot; not synchronized; do not mutate."
Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (4)
5-8: Protocol redesign is minimal and focused.Single read-only snapshot property keeps the surface tight. Consider adding a brief note that it’s a snapshot to discourage retention across async boundaries.
890-898: Confirm availability of Sequence.count(where:) or switch to a portable pattern.Some Swift toolchains lack count(where:). If not universally available in this repo, prefer filter.count.
Apply if needed:
- return data.values.count { entry in - !Self.nosync_isEntryTombstoned(entry, objectsPool: objectsPool) - } + return data.values.lazy + .filter { !Self.nosync_isEntryTombstoned($0, objectsPool: objectsPool) } + .count
900-916: Minor: preallocate result capacity.Saves a few reallocations on large maps.
Apply:
- var result: [(key: String, value: InternalLiveMapValue)] = [] + var result: [(key: String, value: InternalLiveMapValue)] = [] + result.reserveCapacity(data.count)
920-936: Tombstone check logic is clear and centralized.Consider reusing nosync_isEntryTombstoned inside get() for uniformity, but current approach is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift(7 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift(2 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swift(1 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift(1 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swift(1 hunks)Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift(34 hunks)Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectPoolDelegate.swift(0 hunks)Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectsPoolDelegate.swift(1 hunks)Tests/AblyLiveObjectsTests/ObjectsPoolTests.swift(3 hunks)
💤 Files with no reviewable changes (1)
- Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectPoolDelegate.swift
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-26T11:32:54.222Z
Learnt from: CR
PR: ably/ably-liveobjects-swift-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:32:54.222Z
Learning: Applies to Tests/**/*.swift : In tests, import AblyLiveObjects using `testable import AblyLiveObjects`
Applied to files:
Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectsPoolDelegate.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftTests/AblyLiveObjectsTests/ObjectsPoolTests.swift
📚 Learning: 2025-08-26T11:32:26.789Z
Learnt from: CR
PR: ably/ably-liveobjects-swift-plugin#0
File: .cursor/rules/swift.mdc:0-0
Timestamp: 2025-08-26T11:32:26.789Z
Learning: Applies to Sources/AblyLiveObjects/**/*.swift : In AblyLiveObjects library (non-test) code, import modules as: Ably with `import Ably`, and _AblyPluginSupportPrivate with `internal import _AblyPluginSupportPrivate`
Applied to files:
Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftTests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift
📚 Learning: 2025-08-26T11:32:54.222Z
Learnt from: CR
PR: ably/ably-liveobjects-swift-plugin#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-08-26T11:32:54.222Z
Learning: Applies to Tests/**/*.swift : When passing a logger to internal components in tests, use `TestLogger()`
Applied to files:
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Generate code coverage
- GitHub Check: generate-matrices
🔇 Additional comments (15)
Tests/AblyLiveObjectsTests/ObjectsPoolTests.swift (1)
92-92: Rename complete; no stale references remain
Searches forMockLiveMapObjectPoolDelegateandLiveMapObjectPoolDelegatereturned no matches; conformance toLiveMapObjectsPoolDelegateis correctly applied throughout.Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swift (1)
48-50: Type migration to LiveMapObjectsPoolDelegate is consistent with the PR goal.Creation wiring still looks correct, proxy identity semantics unchanged.
Sources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swift (1)
8-9: Good alignment of PublicValueCreationArgs with the new delegate.Mapping to PublicObjectsStore remains unchanged; no functional risk.
Also applies to: 15-17
Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift (1)
18-19: Tests updated to MockLiveMapObjectsPoolDelegate read well and keep intent intact.Wide mechanical rename; assertions and side-effects remain valid.
Also applies to: 37-37, 51-51, 62-62, 75-75, 87-87, 99-99, 113-113, 149-149, 166-166, 181-181, 245-245, 269-269, 313-313, 347-347, 395-395, 433-433, 502-502, 543-543, 619-619, 670-670, 722-722, 749-749, 868-868, 908-908, 934-934, 1035-1035, 1068-1068, 1100-1100, 1149-1149, 1191-1191, 1247-1247
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift (1)
13-16: Delegate type rename is correctly threaded through and used in all accessors.toPublic() calls pass the updated delegate as creation arg; no behavior change.
Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectsPoolDelegate.swift (2)
1-1: Good: uses testable import per our testing convention.Matches the retrieved learning for Tests/**.swift.
8-16: LGTM: deterministic, queue-confined pool seeding.Init composes the mutex and seeds with a clean pool; safe for tests.
Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (3)
5-5: Conformance switch to LiveMapObjectsPoolDelegate looks correct.This aligns with the PR goal to pass the pool directly.
142-148: nosync_objectsPool implementation is appropriate.Returning a copy via withoutLock avoids lock coupling; callers treat it as a snapshot.
5-5: Removed legacy delegate and helper references
Search across the codebase shows no occurrences of the oldLiveMapObjectPoolDelegate,nosync_getObjectFromPool,getObjectFromPool, or related mocks outside of the intended newLiveMapObjectsPoolDelegateimplementation.Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (5)
123-131: get(_:coreSDK:delegate:) correctly threads the pool snapshot.Good lock scoping and channel-state validation.
134-141: size(_:delegate:) path is consistent with get().No issues.
143-150: entries(_:delegate:) correctly filters via helper.Reads from the snapshot only; no hidden synchronization.
152-160: keys()/values() delegation is clean.No concerns.
940-1001: Entry-to-value conversion handles all cases; snapshot dereference is correct.Good early returns for tombstones and missing refs. No issues.
lawrence-forooghian
force-pushed
the
39-less-passing-LiveMap-delegate
branch
from
7f7e807 to
827652c
Compare
github-actions
bot
temporarily deployed
to
staging/pull/88/AblyLiveObjects
lawrence-forooghian
force-pushed
the
48-get-rid-of-InternalError
branch
from
6a61f66 to
5ba838b
Compare
This will subsequently allow us to make more use of value types internally. (The change in this commit, in isolation, actually makes things a bit messier because now we have to mock a whole ObjectsPool, with its root object that the tests don't even need. I suppose we could instead make the delegate just return a dictionary containing the pool's entries, but let's see if it actually becomes a problem.)
It's nicer to deal with value types instead of objects (the latter coming with synchronisation requirements to worry about). Resolves #39.
lawrence-forooghian
force-pushed
the
39-less-passing-LiveMap-delegate
branch
from
827652c to
96799cb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (1)
900-916: entries() should return [] when the map is tombstoned; compactMap can simplify.Mirror get(size)’s tombstone handling for consistency with RTLM semantics.
Apply:
internal func nosync_entries(coreSDK: CoreSDK, objectsPool: ObjectsPool) throws(ARTErrorInfo) -> [(key: String, value: InternalLiveMapValue)] { try coreSDK.nosync_validateChannelState(notIn: [.detached, .failed], operationDescription: "LiveMap.entries") - // RTLM11d: Returns key-value pairs from the internal data map - // RTLM11d1: Pairs with tombstoned entries (per RTLM14) are not returned - var result: [(key: String, value: InternalLiveMapValue)] = [] - for (key, entry) in data where !Self.nosync_isEntryTombstoned(entry, objectsPool: objectsPool) { - // Convert entry to LiveMapValue using the same logic as get(key:) - if let value = nosync_convertEntryToLiveMapValue(entry, objectsPool: objectsPool) { - result.append((key: key, value: value)) - } - } - return result + if liveObjectMutableState.isTombstone { + return [] + } + // RTLM11d: Returns key-value pairs from the internal data map (excluding tombstoned) + return data.compactMap { key, entry -> (key: String, value: InternalLiveMapValue)? in + guard !Self.nosync_isEntryTombstoned(entry, objectsPool: objectsPool) else { return nil } + guard let value = nosync_convertEntryToLiveMapValue(entry, objectsPool: objectsPool) else { return nil } + return (key: key, value: value) + } }
🧹 Nitpick comments (3)
Sources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swift (1)
6-18: Avoid duplicating ACL inside an internal extension.Since the extension is already
internal, the repeatedinternalon members is redundant; prefer putting ACL on the extension only.As per coding guidelines.
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift (1)
25-33: Hoist creationArgs to avoid repeated construction in hot getters.Small readability/micro‑perf win: precompute once and reuse in get/entries/values.
class PublicDefaultLiveMap: LiveMap { ... private let logger: Logger + + private var creationArgs: InternalLiveMapValue.PublicValueCreationArgs { + .init(coreSDK: coreSDK, mapDelegate: delegate, logger: logger) + } ... internal func get(key: String) throws(ARTErrorInfo) -> LiveMapValue? { - try proxied.get(key: key, coreSDK: coreSDK, delegate: delegate)?.toPublic( - creationArgs: .init(coreSDK: coreSDK, mapDelegate: delegate, logger: logger) - ) + try proxied.get(key: key, coreSDK: coreSDK, delegate: delegate)? + .toPublic(creationArgs: creationArgs) } ... internal var entries: [(key: String, value: LiveMapValue)] { get throws(ARTErrorInfo) { - try proxied.entries(coreSDK: coreSDK, delegate: delegate).map { entry in + try proxied.entries(coreSDK: coreSDK, delegate: delegate).map { entry in ( entry.key, - entry.value.toPublic(creationArgs: .init(coreSDK: coreSDK, mapDelegate: delegate, logger: logger)) + entry.value.toPublic(creationArgs: creationArgs) ) } } } ... internal var values: [LiveMapValue] { get throws(ARTErrorInfo) { - try proxied.values(coreSDK: coreSDK, delegate: delegate).map { value in - value.toPublic(creationArgs: .init(coreSDK: coreSDK, mapDelegate: delegate, logger: logger)) - } + try proxied.values(coreSDK: coreSDK, delegate: delegate).map { value in + value.toPublic(creationArgs: creationArgs) + } } }Also applies to: 41-56, 64-76
Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (1)
142-148: Documentnosync_objectsPoolthread-safety contract
Add a doc comment on thenosync_objectsPoolproperty in both theLiveMapObjectsPoolDelegateprotocol and its implementation stating it must be accessed only on the internal queue (enforced bywithoutSync). Verified all production call sites are already withinwithSyncclosures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift(7 hunks)Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift(2 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swift(1 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift(1 hunks)Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swift(1 hunks)Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift(34 hunks)Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectPoolDelegate.swift(0 hunks)Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectsPoolDelegate.swift(1 hunks)Tests/AblyLiveObjectsTests/ObjectsPoolTests.swift(3 hunks)
💤 Files with no reviewable changes (1)
- Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectPoolDelegate.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- Tests/AblyLiveObjectsTests/ObjectsPoolTests.swift
- Tests/AblyLiveObjectsTests/Mocks/MockLiveMapObjectsPoolDelegate.swift
- Tests/AblyLiveObjectsTests/InternalDefaultLiveMapTests.swift
🧰 Additional context used
📓 Path-based instructions (3)
**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)
**/*.swift: Specify an explicit access control level (SwiftLint explicit_acl) for all declarations in Swift code (tests are exempt)
When extending a type, put the access level on the extension declaration rather than on each member (tests are exempt)
Prefer implicit .init(...) when the type can be inferred in initializer expressions
Prefer enum case shorthand (.caseName) when the type can be inferred
For JSONValue or WireValue, prefer using literal syntax via ExpressibleBy*Literal where possible
Prefer Swift raw string literals for JSON strings instead of escaping double quotes
When an array literal begins with an initializer expression, place the initializer on the line after the opening bracket
Files:
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swiftSources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swiftSources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swiftSources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift
Sources/AblyLiveObjects/**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)
In AblyLiveObjects library (non-test) code, import modules as: Ably with
import Ably, and _AblyPluginSupportPrivate withinternal import _AblyPluginSupportPrivate
Files:
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swiftSources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swiftSources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swiftSources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift
Sources/**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
For
testsOnly_property declarations, do not add generic explanatory comments (their meaning is understood)
Files:
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swiftSources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swiftSources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swiftSources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swiftSources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift
🧬 Code graph analysis (2)
Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (1)
Sources/AblyLiveObjects/Utility/DispatchQueueMutex.swift (1)
withoutSync(43-47)
Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (2)
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift (1)
get(25-33)Sources/AblyLiveObjects/Internal/CoreSDK.swift (1)
nosync_validateChannelState(99-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 16.3) - GitHub Check: Xcode, tvOS (Xcode 16.3)
- GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 16.3) - GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 16.3) - GitHub Check: Example app, tvOS (Xcode 16.3)
- GitHub Check: Example app, macOS (Xcode 16.3)
- GitHub Check: Example app, iOS (Xcode 16.3)
- GitHub Check: SPM (Xcode 16.3)
- GitHub Check: Xcode, iOS (Xcode 16.3)
- GitHub Check: Xcode, macOS (Xcode 16.3)
- GitHub Check: SPM,
releaseconfiguration (Xcode 16.3) - GitHub Check: Generate code coverage
🔇 Additional comments (9)
Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicObjectsStore.swift (1)
46-50: Delegate type rename aligned; LGTM.This keeps
MapCreationArgsin sync with the newLiveMapObjectsPoolDelegate.Sources/AblyLiveObjects/Public/Public Proxy Objects/PublicDefaultLiveMap.swift (1)
13-16: Delegate type rename looks correct.Property and initializer now use
LiveMapObjectsPoolDelegate; downstream calls pass it consistently.Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (1)
5-5: Conformance updated to LiveMapObjectsPoolDelegate; LGTM.Matches the project‑wide rename and intended access pattern.
Sources/AblyLiveObjects/Public/Public Proxy Objects/InternalLiveMapValue+ToPublic.swift (1)
8-8: Rename to LiveMapObjectsPoolDelegate is complete; no stale references found. LGTM.Sources/AblyLiveObjects/Internal/InternalDefaultLiveMap.swift (5)
5-7: Delegate snapshot approach looks good.Explicit ACL and Sendable conformance are correct; exposing a read‑only ObjectsPool snapshot aligns with the PR goals. Please ensure implementers return an immutable, cheap snapshot per access.
Can you confirm that all LiveMapObjectsPoolDelegate implementations compute nosync_objectsPool in O(1) without locking, and that it’s documented as a snapshot?
123-131: Getter now pulls a pool snapshot once per call — LGTM.Passing the ObjectsPool snapshot down into nosync_get removes delegate threading without adding locks.
921-936: Missing‑object case in tombstone check — confirm intended semantics.nosync_isEntryTombstoned returns false when the referenced objectId is absent from the pool. That means:
- size() counts that entry (non‑tombstoned), but
- entries() will drop it because conversion returns nil.
If the spec requires size == entries.count, we may need to treat “referenced object missing” as tombstoned for counting purposes.
Would you like me to flip this to treat a missing pool entry as tombstoned for size/entries consistency? I can supply a diff once confirmed.
978-997: Direct pool lookup and LiveObject conversion — LGTM.The switch on poolEntry and pre‑check of nosync_isTombstone align with the snapshot design and avoid delegate indirection.
152-160: keys()/values() delegation — LGTM.Reusing entries() keeps behavior centralized; no additional sync introduced.
Note: This PR is based on top of #87; please review that one first.
It's nicer to deal with value types instead of objects (the latter coming with synchronisation requirements to worry about).
Resolves #39.
Summary by CodeRabbit
Refactor
Tests
Chores