Skip to content

Commit 42dd0c6

Browse files
mlodato517mlodato517
committed
Make functional BST more-functional.
**This Commit** Adds history to the functional BST and no longer requires ownership to `insert`/`delete`. **Why?** Mainly to make benchmarking easier but this has the benefit of more closely matching a functional data structure in that, when new trees are created from "mutating" operations, the history of trees/operations is still accessible. Use struct update syntax for conciseness **Why?** I didn't think I could do this but the issue was that I had `derive`d `Clone` on `Node` when I should've implemented it directly because it's generic and leverages reference counting. See #6 (comment) Explicitly clone needed fields of nodes **This Commit** Removes instances of instantiating new `Node`s using the functional update syntax of a clone of an existing `Node`. That is it replaces: ```rust Node { some_field: some_value, ..existing_node.clone() } ``` with ```rust Node { some_field: some_value, other_field: existing_node.other_field.clone(), // etc. } ``` **Why?** For clarity - when using `..node.clone()` what's happening is we are first cloning the node in its entirety (which results in bumping the reference count for all its fields), moving the fields we care about to the new `Node`, and then dropping the remaining fields (and decrementing their reference count). This is a surprise to the reader and is needless and not what we want to express. It also may have a performance impact but that isn't measured. For more details see [this comment thread][0]. [0]: #6 (comment) Deny `clippy::clone_on_ref_ptr` **This Commit** Denies the use of `.clone()` on reference counted pointers as advised by [this `clippy` lint][0] and [The Rust Book][1]. It also addresses instances where the lint was violated. **Why?** It's best practice to make clear the clone is cheap. There is [some debate][2] on the issue and the [standard library no longer explicitly recommends it][3] but it is still noted as more idiomatic. In any case, for us there are no issues making the change and if there is a possibility for being more expressive, we should take it. See #6 (comment) for more details. [0]: https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_ref_ptr [1]: https://doc.rust-lang.org/book/ch15-04-rc.html#using-rct-to-share-data [2]: rust-lang/rust-clippy#2048 [3]: rust-lang/rust#76138
1 parent f77b68b commit 42dd0c6

File tree

3 files changed

+133
-51
lines changed

3 files changed

+133
-51
lines changed

‎.github/workflows/ci.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,3 +68,18 @@ jobs:
6868
with:
6969
command: clippy
7070
args: -- -D warnings
71+
72+
docs:
73+
name: Rustdoc
74+
runs-on: ubuntu-latest
75+
steps:
76+
- uses: actions/checkout@v2
77+
- uses: actions-rs/toolchain@v1
78+
with:
79+
profile: minimal
80+
toolchain: stable
81+
override: true
82+
- uses: actions-rs/cargo@v1
83+
with:
84+
command: doc
85+
args: --no-deps

‎src/functional.rs

Lines changed: 117 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,38 @@
33
//! expect to modify the tree (e.g. `insert` or `delete`) instead return
44
//! a new tree that reference many of the nodes of the original tree.
55
//!
6-
//! To avoid copious `Rc`ing, we do not implement a particularly efficient
7-
//! persistent structure - we only allow one tree at a time. Still, most
8-
//! of the algorithms are the same and there are useful lessons to learn!
6+
//! # Examples
7+
//!
8+
//! ```
9+
//! use bst::functional::Tree;
10+
//!
11+
//! let tree = Tree::new();
12+
//!
13+
//! // Nothing in here yet.
14+
//! assert_eq!(tree.find(&1), None);
15+
//!
16+
//! // This `insert` returns a new tree!
17+
//! let new_tree = tree.insert(1, 2);
18+
//!
19+
//! // The new tree has this new value but the old one doesn't.
20+
//! assert_eq!(new_tree.find(&1), Some(&2));
21+
//! assert_eq!(tree.find(&1), None);
22+
//!
23+
//! // Insert a new value for the same key gives yet another tree.
24+
//! let newer_tree = new_tree.insert(1, 3);
25+
//!
26+
//! // And delete it for good measure.
27+
//! let newest_tree = newer_tree.delete(&1);
28+
//!
29+
//! // All history is preserved.
30+
//! assert_eq!(newest_tree.find(&1), None);
31+
//! assert_eq!(newer_tree.find(&1), Some(&3));
32+
//! assert_eq!(new_tree.find(&1), Some(&2));
33+
//! assert_eq!(tree.find(&1), None);
34+
//! ```
935
1036
use std::cmp;
37+
use std::rc::Rc;
1138

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

3663
/// Returns a new tree that includes a node
37-
/// containing the given key and value
64+
/// containing the given key and value.
3865
///
3966
/// # Examples
4067
///
4168
/// ```
4269
/// use bst::functional::Tree;
4370
///
44-
/// let mut tree = Tree::new();
45-
/// tree = tree.insert(1, 2);
46-
///
47-
/// assert_eq!(tree.find(&1), Some(&2));
48-
///
49-
/// tree = tree.insert(1, 3);
71+
/// let tree = Tree::new();
72+
/// let new_tree = tree.insert(1, 2);
73+
/// let newer_tree = new_tree.insert(1, 3);
5074
///
51-
/// assert_eq!(tree.find(&1), Some(&3));
75+
/// // All history is preserved.
76+
/// assert_eq!(newer_tree.find(&1), Some(&3));
77+
/// assert_eq!(new_tree.find(&1), Some(&2));
78+
/// assert_eq!(tree.find(&1), None);
5279
/// ```
53-
pub fn insert(self, key: K, value: V) -> Self
80+
pub fn insert(&self, key: K, value: V) -> Self
5481
where
5582
K: cmp::Ord,
5683
{
5784
match self {
5885
Tree::Leaf => Tree::Node(Node::new(key, value)),
5986
Tree::Node(n) => match key.cmp(&n.key) {
6087
cmp::Ordering::Less => Tree::Node(Node {
61-
left: Box::new(n.left.insert(key, value)),
62-
..n
88+
left: Rc::new(n.left.insert(key, value)),
89+
key: Rc::clone(&n.key),
90+
right: Rc::clone(&n.right),
91+
value: Rc::clone(&n.value),
92+
}),
93+
cmp::Ordering::Equal => Tree::Node(Node {
94+
value: Rc::new(value),
95+
key: Rc::clone(&n.key),
96+
left: Rc::clone(&n.left),
97+
right: Rc::clone(&n.right),
6398
}),
64-
cmp::Ordering::Equal => Tree::Node(Node { value, ..n }),
6599
cmp::Ordering::Greater => Tree::Node(Node {
66-
right: Box::new(n.right.insert(key, value)),
67-
..n
100+
right: Rc::new(n.right.insert(key, value)),
101+
key: Rc::clone(&n.key),
102+
left: Rc::clone(&n.left),
103+
value: Rc::clone(&n.value),
68104
}),
69105
},
70106
}
@@ -79,8 +115,8 @@ impl<K, V> Tree<K, V> {
79115
/// ```
80116
/// use bst::functional::Tree;
81117
///
82-
/// let mut tree = Tree::new();
83-
/// tree = tree.insert(1, 2);
118+
/// let tree = Tree::new();
119+
/// let tree = tree.insert(1, 2);
84120
///
85121
/// assert_eq!(tree.find(&1), Some(&2));
86122
/// assert_eq!(tree.find(&42), None);
@@ -109,86 +145,117 @@ impl<K, V> Tree<K, V> {
109145
/// ```
110146
/// use bst::functional::Tree;
111147
///
112-
/// let mut tree = Tree::new();
113-
/// tree = tree.insert(1, 2);
114-
///
115-
/// tree = tree.delete(&1);
148+
/// let tree = Tree::new();
149+
/// let tree = tree.insert(1, 2);
150+
/// let newer_tree = tree.delete(&1);
116151
///
117-
/// assert_eq!(tree.find(&1), None);
152+
/// // All history is preserved.
153+
/// assert_eq!(newer_tree.find(&1), None);
154+
/// assert_eq!(tree.find(&1), Some(&2));
118155
/// ```
119-
pub fn delete(self, k: &K) -> Self
156+
pub fn delete(&self, k: &K) -> Self
120157
where
121158
K: cmp::Ord,
122159
{
123160
match self {
124-
Tree::Leaf => self,
161+
Tree::Leaf => Tree::Leaf,
125162
Tree::Node(n) => match k.cmp(&n.key) {
126163
cmp::Ordering::Less => Tree::Node(Node {
127-
left: Box::new(n.left.delete(k)),
128-
..n
164+
left: Rc::new(n.left.delete(k)),
165+
key: Rc::clone(&n.key),
166+
right: Rc::clone(&n.right),
167+
value: Rc::clone(&n.value),
129168
}),
130-
cmp::Ordering::Equal => match (*n.left, *n.right) {
131-
(Tree::Leaf, right_child) => right_child,
132-
(left_child, Tree::Leaf) => left_child,
169+
cmp::Ordering::Equal => match (n.left.as_ref(), n.right.as_ref()) {
170+
(Tree::Leaf, Tree::Leaf) => Tree::Leaf,
171+
(Tree::Leaf, Tree::Node(right)) => Tree::Node(right.clone()),
172+
(Tree::Node(left), Tree::Leaf) => Tree::Node(left.clone()),
133173

134174
// If we have two children we have to figure out
135175
// which node to promote. We choose here this node's
136176
// predecessor. That is, the largest node in this node's
137177
// left subtree.
138-
(Tree::Node(left_child), right_child) => {
178+
(Tree::Node(left_child), _) => {
139179
let (pred_key, pred_val, new_left) = left_child.delete_largest();
140180
Tree::Node(Node {
141181
left: new_left,
142-
right: Box::new(right_child), // I really don't want this allocation here
182+
right: Rc::clone(&n.right),
143183
key: pred_key,
144184
value: pred_val,
145185
})
146186
}
147187
},
148188
cmp::Ordering::Greater => Tree::Node(Node {
149-
right: Box::new(n.right.delete(k)),
150-
..n
189+
right: Rc::new(n.right.delete(k)),
190+
key: Rc::clone(&n.key),
191+
left: Rc::clone(&n.left),
192+
value: Rc::clone(&n.value),
151193
}),
152194
},
153195
}
154196
}
155197
}
156198

157-
/// A `Node` tree has a key that is used for searching/sorting and a value
199+
/// A `Node` has a key that is used for searching/sorting and a value
158200
/// that is associated with that key. It always has two children although
159201
/// those children may be [`Leaf`][Tree::Leaf]s.
160202
pub struct Node<K, V> {
161-
key: K,
162-
value: V,
163-
left: Box<Tree<K, V>>,
164-
right: Box<Tree<K, V>>,
203+
key: Rc<K>,
204+
value: Rc<V>,
205+
left: Rc<Tree<K, V>>,
206+
right: Rc<Tree<K, V>>,
207+
}
208+
209+
/// Manual implementation of `Clone` so we don't clone references when the generic parameters
210+
/// aren't `Clone` themselves.
211+
///
212+
/// Note the comment on generic structs in
213+
/// [the docs][<https://doc.rust-lang.org/std/clone/trait.Clone.html#derivable>].
214+
impl<K, V> Clone for Node<K, V> {
215+
fn clone(&self) -> Self {
216+
Self {
217+
key: Rc::clone(&self.key),
218+
left: Rc::clone(&self.left),
219+
right: Rc::clone(&self.right),
220+
value: Rc::clone(&self.value),
221+
}
222+
}
165223
}
166224

167225
impl<K, V> Node<K, V> {
226+
/// Construct a new `Node` with the given `key` and `value.
168227
fn new(key: K, value: V) -> Self {
169228
Self {
170-
key,
171-
value,
172-
left: Box::new(Tree::Leaf),
173-
right: Box::new(Tree::Leaf),
229+
key: Rc::new(key),
230+
value: Rc::new(value),
231+
left: Rc::new(Tree::Leaf),
232+
right: Rc::new(Tree::Leaf),
174233
}
175234
}
176235

177-
/// Returns the largest node and a new subtree
178-
/// without that largest node.
179-
fn delete_largest(self) -> (K, V, Box<Tree<K, V>>)
236+
/// Returns the key and value of the largest node and a new subtree without that largest node.
237+
fn delete_largest(&self) -> (Rc<K>, Rc<V>, Rc<Tree<K, V>>)
180238
where
181239
K: cmp::Ord,
182240
{
183-
match *self.right {
184-
Tree::Leaf => (self.key, self.value, self.left),
241+
match self.right.as_ref() {
242+
Tree::Leaf => (
243+
Rc::clone(&self.key),
244+
Rc::clone(&self.value),
245+
Rc::clone(&self.left),
246+
),
185247
Tree::Node(r) => {
186248
let (key, value, sub) = r.delete_largest();
187249

188250
(
189251
key,
190252
value,
191-
Box::new(Tree::Node(Node { right: sub, ..self })),
253+
Rc::new(Tree::Node(Node {
254+
right: sub,
255+
key: Rc::clone(&self.key),
256+
left: Rc::clone(&self.left),
257+
value: Rc::clone(&self.value),
258+
})),
192259
)
193260
}
194261
}

‎src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@
2323
//! in the tree. BSTs also naturally support sorted iteration by visiting the
2424
//! left subtree, then the subtree root, then the right subtree.
2525
26-
#![deny(missing_docs)]
26+
#![deny(missing_docs, clippy::clone_on_ref_ptr)]
2727

2828
pub mod functional;

0 commit comments

Comments
 (0)