Skip to content
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.

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

fix: handle first key in subsets #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 52 additions & 10 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
#![warn(
clippy::all,
clippy::pedantic,
clippy::nursery,
clippy::cargo,
missing_docs
)]
#![warn(clippy::all, clippy::cargo, missing_docs)]
//! Fast subset and superset queries based on tries.
//!
//! ```rust
Expand Down Expand Up @@ -55,7 +49,7 @@ impl<K, T> Drop for Node<K, T> {
stack.push(child);
while let Some(mut current) = stack.pop() {
while let Some((_, child)) = current.children.pop() {
stack.push(child)
stack.push(child);
}
}
}
Expand Down Expand Up @@ -177,7 +171,7 @@ where
/// assert_eq!(trie.values().collect::<Vec<_>>(), vec![&"foo", &"bar", &"baz"]);
/// ```
#[must_use]
pub fn values(&self) -> Values<K, T> {
pub const fn values(&self) -> Values<K, T> {
Values::new(self)
}

Expand Down Expand Up @@ -212,7 +206,7 @@ where
{
fn extend<F: IntoIterator<Item = (I, T)>>(&mut self, iter: F) {
for (k, t) in iter {
self.insert(k, t)
self.insert(k, t);
}
}
}
Expand Down Expand Up @@ -259,4 +253,52 @@ mod tests {
current = current.entry(i - 1..i).or_insert(i)
}
}

#[test]
// https://github.com/KaiserKarel/set-trie/issues/6
fn subsets_small2() {
let mut v = SetTrie::new();
v.insert(&[1, 2], 'a');
{
let mut s = v.subsets(&[&0, &1]);
assert_eq!(s.next(), None);
}
{
let mut s = v.subsets(&[&0, &1, &2]);
assert_eq!(s.next(), None);
}
{
let mut s = v.subsets(&[&1, &2]);
assert_eq!(s.next(), Some(&'a'));
}
{
v.insert(&[0, 2], 'a');
let mut s = v.subsets(&[&0, &2]);
assert_eq!(s.next(), Some(&'a'));
}
}

#[test]
// https://github.com/KaiserKarel/set-trie/issues/6
fn supersets_small2() {
let mut v = SetTrie::new();
v.insert(&[1, 2], 'a');
{
let mut s = v.supersets(&[&0]);
assert_eq!(s.next(), None);
}
{
let mut s = v.supersets(&[]);
assert_eq!(s.next(), Some(&'a'));
}
{
let mut s = v.supersets(&[&1]);
assert_eq!(s.next(), Some(&'a'));
}
{
v.insert(&[0, 2], 'a');
let mut s = v.supersets(&[&1, &2]);
assert_eq!(s.next(), Some(&'a'));
}
}
}
52 changes: 41 additions & 11 deletions src/subset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use crate::{Node, SetTrie};
/// Iterator for [subset](SetTrie::subset) method.
#[derive(Debug, Clone)]
pub struct Subset<'a, 'b, K, T> {
current: &'a Node<K, T>,
current: Option<&'a Node<K, T>>,

/// Buffer of next nodes to visit.
next: Vec<(&'a K, &'a Node<K, T>)>,
idx: usize,
keys: &'b [K],
Expand All @@ -13,9 +15,33 @@ impl<'a, 'b, K, T> Subset<'a, 'b, K, T>
where
K: Ord,
{
#[must_use]
pub(crate) fn new(trie: &'a SetTrie<K, T>, keys: &'b [K]) -> Self {
// There might be a cleaner way to accomplish this. Right now we're doing
// computation in the subset iterator, which means it's not fully lazy.
let current = match keys.len() {
// Empty keys has it's own leaves as childeren as items.
0 => Some(&trie.0),
_ => {
if let Some(first) = keys.first() {
if trie
.0
.children
.binary_search_by(|(child, _)| child.cmp(first))
.is_ok()
{
Some(&trie.0)
} else {
None
}
} else {
Some(&trie.0)
}
}
};

Subset {
current: &trie.0,
current,
next: vec![],
idx: 0,
keys,
Expand All @@ -30,15 +56,18 @@ where
type Item = &'a T;

fn next(&mut self) -> Option<Self::Item> {
if self.idx < self.current.leaves.len() {
self.current?;

if self.idx < self.current.unwrap().leaves.len() {
self.idx += 1;
Some(&self.current.leaves[self.idx - 1])
Some(&self.current.unwrap().leaves[self.idx - 1])
} else {
if let (Some(from), Some(to)) = (self.keys.first(), self.keys.last()) {
self.next.extend(
self.current
.unwrap()
// technically, between inclusive is only necessary on the first iteration,
// where we check handle the root node. Every subsequent iter may use up-to,
// where we handle the root node. Every subsequent iter may use up-to,
// which saves a single binary search. For long key lengths this may matter.
.between_inclusive(from, to)
.iter()
Expand All @@ -49,22 +78,23 @@ where
while let Some((k, node)) = self.next.pop() {
if self.keys.binary_search(k).is_ok() {
self.idx = 0;
self.current = node;
self.current = Some(node);
return self.next();
}
self.next.extend(
node.between_inclusive(from, to)
.iter()
.map(|n| (&n.0, &n.1)),
)
);
}
}
None
}
}

fn size_hint(&self) -> (usize, Option<usize>) {
(self.current.leaves.len() - self.idx, None)
self.current
.map_or((0, None), |current| (current.leaves.len() - self.idx, None))
}
}

Expand Down Expand Up @@ -94,11 +124,11 @@ mod tests {
// A set is its own subset.
assert_eq!(v.subsets(&[]).collect::<Vec<_>>(), vec![&'f']);

// Quite a specific match should work.
// // Quite a specific match should work.
assert_eq!(v.subsets(&[&5]).collect::<Vec<_>>(), vec![&'f', &'i']);

// Non-existing key should match nothing
assert_eq!(v.subsets(&[&6]).collect::<Vec<_>>(), vec![&'f']);
// // Non-existing key should match nothing
assert_eq!(v.subsets(&[&6]).collect::<Vec<&char>>().len(), 0);
}

mod proptest {
Expand Down