Skip to content

+OrderedSet add filter #158 #159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 17, 2022
Merged

+OrderedSet add filter #158 #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6142869
+OrderedSet add filter #158
ktoso Aug 16, 2022
f8726ce
checkInvariants
ktoso Aug 17, 2022
4138ccd
improve test to cover more cases
ktoso Aug 17, 2022
876586f
expect type is correct
ktoso Aug 17, 2022
74a800a
Apply suggestions from code review
ktoso Aug 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Sources/OrderedCollections/OrderedSet/OrderedSet.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,3 +494,27 @@ extension OrderedSet {
return _elements.remove(at: index)
}
}

extension OrderedSet {
/// Returns a new ordered set containing the values pairs of the ordered set
/// that satisfy the given predicate.
///
/// - Parameter isIncluded: A closure that takes a value as its
/// argument and returns a Boolean value indicating whether the value
/// should be included in the returned dictionary.
///
/// - Returns: An ordered set of the values that `isIncluded` allows.
///
/// - Complexity: O(`count`)
@inlinable
public func filter(
_ isIncluded: (Element) throws -> Bool
) rethrows -> Self {
Copy link
Member

@lorentey lorentey Aug 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation can be improved by using a temporary bitset, as in Set.filter -- I expect this will boost performance by 2x or more, by eliminating the need to repeatedly resize the hash table.

(This does not need to be implemented before this lands.)

Copy link
Member Author

@ktoso ktoso Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I poked a little bit at it but we end up having to pull in quite a bunch of new uncheckedElement, _bucketCounts etc I'm not super sure if I'm implementing them right -- so leaving this as future work I guess. Happy to make a ticket for it after this lands.

Copy link
Member

@lorentey lorentey Aug 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the stdlib code wouldn't directly translate -- that's just illustrating the concept. We have a separate bitset implementation within this code base, so we'd need to adapt the algorithm to use that instead. (OrderedSet.symmetricDifference is an example.)

I'm happy to do this as a followup.

Copy link
Member Author

@ktoso ktoso Aug 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup: #163

var result: OrderedSet = Self(minimumCapacity: _minimumCapacity)
for element in self where try isIncluded(element) {
result._appendNew(element)
}
result._checkInvariants()
return result
}
}
4 changes: 4 additions & 0 deletions Sources/_CollectionsTestSupport/AssertionContexts/Assertions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,3 +402,7 @@ public func expectThrows<T>(
errorHandler(error)
}
}

public func expectType<T>(_ actual: T, _ expectedType: Any.Type) {
expectTrue(type(of: actual) == expectedType)
}
20 changes: 20 additions & 0 deletions Tests/OrderedCollectionsTests/OrderedSet/OrderedSetTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1360,4 +1360,24 @@ class OrderedSetTests: CollectionTestCase {
}
}
}

func test_filter() {
withOrderedSetLayouts(scales: [0, 5, 6]) { layout in
withEvery("factor", in: [1, 2, 3, 5, 10]) { factor in
let count = layout.count
let input = OrderedSet(layout: layout, contents: 0 ..< count)
let expected = (0 ..< count).filter { $0 % factor == 0 }
let actual = input.filter { $0 % factor == 0 }

expectEqualElements(actual, expected)
}
}
}
func test_filter_type() {
let s = Set([1, 2, 3, 4]).filter { $0.isMultiple(of: 2) }
expectType(s, Set<Int>.self)

let os = OrderedSet([1, 2, 3, 4]).filter { $0.isMultiple(of: 2) }
expectType(os, OrderedSet<Int>.self)
}
}