Skip to content
Merged
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
15 changes: 15 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,18 @@ jobs:
with:
command: clippy
args: -- -D warnings

docs:
name: Rustdoc
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true
- uses: actions-rs/cargo@v1
with:
command: doc
args: --no-deps
167 changes: 117 additions & 50 deletions src/functional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,38 @@
//! expect to modify the tree (e.g. `insert` or `delete`) instead return
//! a new tree that reference many of the nodes of the original tree.
//!
//! To avoid copious `Rc`ing, we do not implement a particularly efficient
//! persistent structure - we only allow one tree at a time. Still, most
//! of the algorithms are the same and there are useful lessons to learn!
//! # Examples
//!
//! ```
//! use bst::functional::Tree;
Copy link
Owner Author

@mlodato517 mlodato517 Oct 21, 2021

Choose a reason for hiding this comment

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

This is sort of duplicative with other doctests but it makes for nice docs.

//!
//! let tree = Tree::new();
//!
//! // Nothing in here yet.
//! assert_eq!(tree.find(&1), None);
//!
//! // This `insert` returns a new tree!
//! let new_tree = tree.insert(1, 2);
//!
//! // The new tree has this new value but the old one doesn't.
//! assert_eq!(new_tree.find(&1), Some(&2));
//! assert_eq!(tree.find(&1), None);
//!
//! // Insert a new value for the same key gives yet another tree.
//! let newer_tree = new_tree.insert(1, 3);
//!
//! // And delete it for good measure.
//! let newest_tree = newer_tree.delete(&1);
//!
//! // All history is preserved.
//! assert_eq!(newest_tree.find(&1), None);
//! assert_eq!(newer_tree.find(&1), Some(&3));
//! assert_eq!(new_tree.find(&1), Some(&2));
//! assert_eq!(tree.find(&1), None);
//! ```

use std::cmp;
use std::rc::Rc;

/// A Binary Search Tree. This can be used for inserting, finding,
/// and deleting keys and values. Note that this data structure is
Expand All @@ -34,37 +61,46 @@ impl<K, V> Tree<K, V> {
}

/// Returns a new tree that includes a node
/// containing the given key and value
/// containing the given key and value.
///
/// # Examples
///
/// ```
/// use bst::functional::Tree;
///
/// let mut tree = Tree::new();
/// tree = tree.insert(1, 2);
///
/// assert_eq!(tree.find(&1), Some(&2));
///
/// tree = tree.insert(1, 3);
/// let tree = Tree::new();
/// let new_tree = tree.insert(1, 2);
/// let newer_tree = new_tree.insert(1, 3);
///
/// assert_eq!(tree.find(&1), Some(&3));
/// // All history is preserved.
/// assert_eq!(newer_tree.find(&1), Some(&3));
/// assert_eq!(new_tree.find(&1), Some(&2));
/// assert_eq!(tree.find(&1), None);
/// ```
pub fn insert(self, key: K, value: V) -> Self
pub fn insert(&self, key: K, value: V) -> Self
where
K: cmp::Ord,
{
match self {
Tree::Leaf => Tree::Node(Node::new(key, value)),
Tree::Node(n) => match key.cmp(&n.key) {
cmp::Ordering::Less => Tree::Node(Node {
left: Box::new(n.left.insert(key, value)),
..n
left: Rc::new(n.left.insert(key, value)),
key: Rc::clone(&n.key),
right: Rc::clone(&n.right),
value: Rc::clone(&n.value),
}),
cmp::Ordering::Equal => Tree::Node(Node {
value: Rc::new(value),
key: Rc::clone(&n.key),
left: Rc::clone(&n.left),
right: Rc::clone(&n.right),
}),
cmp::Ordering::Equal => Tree::Node(Node { value, ..n }),
cmp::Ordering::Greater => Tree::Node(Node {
right: Box::new(n.right.insert(key, value)),
..n
right: Rc::new(n.right.insert(key, value)),
key: Rc::clone(&n.key),
left: Rc::clone(&n.left),
value: Rc::clone(&n.value),
}),
},
}
Expand All @@ -79,8 +115,8 @@ impl<K, V> Tree<K, V> {
/// ```
/// use bst::functional::Tree;
///
/// let mut tree = Tree::new();
/// tree = tree.insert(1, 2);
/// let tree = Tree::new();
/// let tree = tree.insert(1, 2);
///
/// assert_eq!(tree.find(&1), Some(&2));
/// assert_eq!(tree.find(&42), None);
Expand Down Expand Up @@ -109,86 +145,117 @@ impl<K, V> Tree<K, V> {
/// ```
/// use bst::functional::Tree;
///
/// let mut tree = Tree::new();
/// tree = tree.insert(1, 2);
///
/// tree = tree.delete(&1);
/// let tree = Tree::new();
/// let tree = tree.insert(1, 2);
/// let newer_tree = tree.delete(&1);
///
/// assert_eq!(tree.find(&1), None);
/// // All history is preserved.
/// assert_eq!(newer_tree.find(&1), None);
/// assert_eq!(tree.find(&1), Some(&2));
/// ```
pub fn delete(self, k: &K) -> Self
pub fn delete(&self, k: &K) -> Self
where
K: cmp::Ord,
{
match self {
Tree::Leaf => self,
Copy link
Owner Author

Choose a reason for hiding this comment

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

We now have to return an owned Self and self is a borrowed Self. We could add a Clone implementation to Tree (I think we'll want it later anyway) but this seems fine for now.

Tree::Leaf => Tree::Leaf,
Tree::Node(n) => match k.cmp(&n.key) {
cmp::Ordering::Less => Tree::Node(Node {
left: Box::new(n.left.delete(k)),
..n
left: Rc::new(n.left.delete(k)),
key: Rc::clone(&n.key),
right: Rc::clone(&n.right),
value: Rc::clone(&n.value),
}),
cmp::Ordering::Equal => match (*n.left, *n.right) {
(Tree::Leaf, right_child) => right_child,
(left_child, Tree::Leaf) => left_child,
cmp::Ordering::Equal => match (n.left.as_ref(), n.right.as_ref()) {
(Tree::Leaf, Tree::Leaf) => Tree::Leaf,
(Tree::Leaf, Tree::Node(right)) => Tree::Node(right.clone()),
(Tree::Node(left), Tree::Leaf) => Tree::Node(left.clone()),
Comment on lines +171 to +172
Copy link
Collaborator

Choose a reason for hiding this comment

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

More just curious - are these 2 .clone() calls intentionally not Rc::clone()?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Correct - right and left are of type Node instead of Rc so we can't use that syntax. We could use Node::clone(left) but that isn't obviously clearer (in my opinion).


// If we have two children we have to figure out
// which node to promote. We choose here this node's
// predecessor. That is, the largest node in this node's
// left subtree.
(Tree::Node(left_child), right_child) => {
(Tree::Node(left_child), _) => {
let (pred_key, pred_val, new_left) = left_child.delete_largest();
Tree::Node(Node {
left: new_left,
right: Box::new(right_child), // I really don't want this allocation here
right: Rc::clone(&n.right),
key: pred_key,
value: pred_val,
})
}
},
cmp::Ordering::Greater => Tree::Node(Node {
right: Box::new(n.right.delete(k)),
..n
right: Rc::new(n.right.delete(k)),
key: Rc::clone(&n.key),
left: Rc::clone(&n.left),
value: Rc::clone(&n.value),
}),
},
}
}
}

/// A `Node` tree has a key that is used for searching/sorting and a value
/// A `Node` has a key that is used for searching/sorting and a value
/// that is associated with that key. It always has two children although
/// those children may be [`Leaf`][Tree::Leaf]s.
pub struct Node<K, V> {
key: K,
value: V,
left: Box<Tree<K, V>>,
right: Box<Tree<K, V>>,
key: Rc<K>,
value: Rc<V>,
Copy link
Owner Author

@mlodato517 mlodato517 Oct 21, 2021

Choose a reason for hiding this comment

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

I'm conflicted here. I have this here because of how delete_largest works. It returns an owned key and value and we can't do that unless we Rc each individual field. I guess it could instead return Rc<Node>? I'm guessing we'd basically clone the predecessor node and return that. But it'd have to have a different subtree and we can't mutate it 🤔

Going to think on it more but I'm also guessing one of Chris's patented Good Questions™️ will clear this up presently.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm still going to wait for Chris's wisdom but I'm semi-convinced this is correct (reference counting all the fields of the Node, that is) - if a new Node is inserted with the same key as another, we must make a new node that references the key and subtrees of the original Node and I think that's only possible by cloning each field.

left: Rc<Tree<K, V>>,
right: Rc<Tree<K, V>>,
}

/// Manual implementation of `Clone` so we don't clone references when the generic parameters
/// aren't `Clone` themselves.
///
/// Note the comment on generic structs in
/// [the docs][<https://doc.rust-lang.org/std/clone/trait.Clone.html#derivable>].
impl<K, V> Clone for Node<K, V> {
fn clone(&self) -> Self {
Self {
key: Rc::clone(&self.key),
left: Rc::clone(&self.left),
right: Rc::clone(&self.right),
value: Rc::clone(&self.value),
}
}
}

impl<K, V> Node<K, V> {
/// Construct a new `Node` with the given `key` and `value.
fn new(key: K, value: V) -> Self {
Self {
key,
value,
left: Box::new(Tree::Leaf),
right: Box::new(Tree::Leaf),
key: Rc::new(key),
value: Rc::new(value),
left: Rc::new(Tree::Leaf),
right: Rc::new(Tree::Leaf),
}
}

/// Returns the largest node and a new subtree
/// without that largest node.
fn delete_largest(self) -> (K, V, Box<Tree<K, V>>)
/// Returns the key and value of the largest node and a new subtree without that largest node.
fn delete_largest(&self) -> (Rc<K>, Rc<V>, Rc<Tree<K, V>>)
where
K: cmp::Ord,
{
match *self.right {
Tree::Leaf => (self.key, self.value, self.left),
match self.right.as_ref() {
Tree::Leaf => (
Rc::clone(&self.key),
Rc::clone(&self.value),
Rc::clone(&self.left),
),
Tree::Node(r) => {
let (key, value, sub) = r.delete_largest();

(
key,
value,
Box::new(Tree::Node(Node { right: sub, ..self })),
Rc::new(Tree::Node(Node {
right: sub,
key: Rc::clone(&self.key),
left: Rc::clone(&self.left),
value: Rc::clone(&self.value),
})),
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@
//! in the tree. BSTs also naturally support sorted iteration by visiting the
//! left subtree, then the subtree root, then the right subtree.

#![deny(missing_docs)]
#![deny(missing_docs, clippy::clone_on_ref_ptr)]

pub mod functional;