Added merge, filter and other functionalities in ThreadSafeDictionary.#1112
Conversation
4b9ab6b to
22326eb
Compare
cdhoffmann
left a comment
There was a problem hiding this comment.
Can you clarify on the PR description what these functions are needed for specifically.
Add: Added json files
| return filteredDictionary | ||
| } | ||
|
|
||
| @inlinable public func contains(where predicate: (Element) -> Bool) -> Bool { |
There was a problem hiding this comment.
Can you add comments for all these functions?
| } | ||
| } | ||
|
|
||
| @inlinable public func filter(_ isIncluded: (Element) -> Bool) -> ThreadSafeDictionary<K, V> { |
There was a problem hiding this comment.
I recommend removing this function entirely and handling filtering within optimize.
This function may lead to incorrect usage, such as modifying a thread safe dictionary in place:
threadSafeDict = threadSafeDict.filter(..)
If the goal is only to filter, return a copy of the filtered items instead.
There was a problem hiding this comment.
I feel it would be better to return ThreadSafeDictionary itself while applying filter operation on it. However, the need was to get a dictionary in return. I have modified it to return a dictionary after your concern. Please let me know if this looks better?
|
|
||
| } | ||
|
|
||
| extension ThreadSafeDictionary: Codable where K: Codable, V: Codable { |
There was a problem hiding this comment.
Why is this needed? Is Optimize serializing thread safe dictionary?
There was a problem hiding this comment.
We need to send back the dictionary in event data, but since we can get the shallow copy from thread safe dictionary we don't need this. And thus i have removed it. Thanks a lot for pointing this.
|
@cdhoffmann, @praveek Thanks for reviewing. I have made relevant changes and would request you to verify them. |
| @inlinable public func filter(_ isIncluded: (Element) -> Bool) -> [K: V] { | ||
| var filteredDictionary = [K: V]() | ||
| queue.sync { | ||
| for (key, value) in dictionary where isIncluded((key, value)) { |
There was a problem hiding this comment.
nit: You can use dictionary.filter(..) method here.
| dict.merge([1: "One", 2: "Two", 3: "Three", 4: "Four"]) { _, new in new } | ||
|
|
||
| // Apply filter to keep only even keys | ||
| var filtered = dict.filter { key, _ in key % 2 == 0 } |
There was a problem hiding this comment.
This test is not needed as filter returns a regular dictionary.
There was a problem hiding this comment.
Updated test.
Description
This PR extends ThreadSafeDictionary with utility functions like filter, contains(where:), removeAll(), and merge(_:uniquingKeysWith:) to align with Dictionary capabilities while ensuring thread safety.
These additions are necessary for the Optimize extension, where we are replacing Dictionary with ThreadSafeDictionary to maintain concurrency safety.
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: