Red black trees #288
Red black trees #288
Conversation
First implementation of Ordered Map and Ordered Set
Small changes to address my mistakes in naming.
First implementation of queues based on implementation of Hood Melville queues from "Purely Functional Data Structures" Chris Okasaki.
Little correction of definitions.
Final touches before merge
… other things to understand what is going on in RedBlackTree
MinionJakub
force-pushed
the
RedBlackTrees
branch
from
1b8dfcb to
2e8cf76
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements Red-Black Tree data structures and a Hood-Melville Queue as part of incremental changes to maps and sets implementation. It introduces low-level tree manipulation functions for maintaining red-black tree invariants, along with a persistent queue implementation and supporting test coverage.
Key changes:
- Adds
Orderedtype (Lt | Eq | Gt) to Base/Types.fram for comparison operations - Implements a complete Red-Black Tree module with insertion, deletion, splitting, and joining operations
- Adds Hood-Melville Queue implementation with O(1) amortized operations
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
| lib/Base/Types.fram | Adds Ordered data type for comparison results |
| lib/RedBlackTree.fram | Complete red-black tree implementation with zipper-based operations for maintaining tree invariants |
| lib/Queue.fram | Hood-Melville queue implementation using incremental rotation state |
| test/stdlib/stdlib0005_Queue.fram | Basic functional tests for Queue operations (push, pop, head, isEmpty, fromList, toList) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub let rec split compareWithPivot tree = | ||
| match tree with | ||
| | Leaf => (None,Leaf,Leaf) | ||
| | Node {left,value,right} => |
There was a problem hiding this comment.
Inconsistent spacing in record pattern. For consistency with the rest of the file where spaces are used after commas in record patterns, this should be {left, value, right} instead of {left,value,right}.
| | Node {left,value,right} => | |
| | Node {left, value, right} => |
| end | ||
| end | ||
|
|
||
| # serachMin finds smallest element in a tree and builds zipper |
There was a problem hiding this comment.
Typo in comment: "serachMin" should be "searchMin".
|
|
||
| # Constructor for node | ||
| pub let construct color size left value right = | ||
| Node {color,size,left,value,right} |
There was a problem hiding this comment.
Inconsistent spacing in record construction. For consistency with line 48 where Node {color, size = size left + size right + 1, left, value, right} uses spaces after commas, this should be Node {color, size, left, value, right} instead of Node {color,size,left,value,right}.
| Node {color,size,left,value,right} | |
| Node {color, size, left, value, right} |
|
|
||
| # The father is red and no nephew is red. | ||
| # Then father becomes black and brother becomes red | ||
| # removing therfore the inequality of blackness. |
There was a problem hiding this comment.
Typo in comment: "therfore" should be "therefore".
| pub let rec blackHeight tree acc = | ||
| match tree with | ||
| | Leaf => acc | ||
| | Node {color=Red,left} => blackHeight left acc |
There was a problem hiding this comment.
Inconsistent spacing in record pattern. For consistency with the rest of the file (lines 94, 105, 116, 126, 196, 210, 226, 243, 286, 293, 388), this should use spaces around = like {color = Red, left} instead of {color=Red,left}.
| | Node {color=Red,left} => blackHeight left acc | |
| | Node {color = Red, left} => blackHeight left acc |
| match searchMax left [] with | ||
| | Right color leftSmall value :: zipper => | ||
| joinVal (deleteNearLeaf color leftSmall zipper) value right | ||
| |_ => left |
There was a problem hiding this comment.
Missing space after pipe. For consistency with other pattern matches in the file (e.g., line 445 | Leaf => right), this should be | _ => left instead of |_ => left.
| |_ => left | |
| | _ => left |
| match tree with | ||
| | Leaf => zipper | ||
| | Node {color, left, value, right} => | ||
| searchMax right (Right color left value:: zipper) |
There was a problem hiding this comment.
Missing space before :: operator. For consistency with line 314 and 324, this should be value :: zipper instead of value:: zipper.
| searchMax right (Right color left value:: zipper) | |
| searchMax right (Right color left value :: zipper) |
| searchHeight leftward target right (Right Red left value :: zipper) | ||
| | Node {color=Black,left,value,right} => | ||
| if 0 == target then | ||
| (tree,zipper) |
There was a problem hiding this comment.
Inconsistent spacing in tuple. For consistency with line 407 and 308 where tuples are written as (Leaf, zipper) with space after comma, this should be (tree, zipper) instead of (tree,zipper).
| (tree,zipper) | |
| (tree, zipper) |
| # Splits tree according to the function | ||
| pub let rec split compareWithPivot tree = | ||
| match tree with | ||
| | Leaf => (None,Leaf,Leaf) |
There was a problem hiding this comment.
Inconsistent spacing in tuple. For consistency with other tuples in the file where spacing is used (e.g., line 407 (Leaf, zipper)), this should be (None, Leaf, Leaf) instead of (None,Leaf,Leaf).
| | Leaf => (None,Leaf,Leaf) | |
| | Leaf => (None, Leaf, Leaf) |
| searchHeight leftward target left (Left Red value right :: zipper) | ||
| else | ||
| searchHeight leftward target right (Right Red left value :: zipper) | ||
| | Node {color=Black,left,value,right} => |
There was a problem hiding this comment.
Inconsistent spacing in record pattern. For consistency with the rest of the file, this should use spaces around = and after , like {color = Black, left, value, right} instead of {color=Black,left,value,right}.
| | Node {color=Black,left,value,right} => | |
| | Node {color = Black, left, value, right} => |
4 participants
Incremental changes to implementation of maps and sets. This one is about RedBlack Tree. This is continuation of Pull Request #136 .