From dfd9b31b578e641e2f6384b21dbe6b94db91b219 Mon Sep 17 00:00:00 2001 From: luc-blaeser Date: Mon, 13 Feb 2023 19:53:31 +0100 Subject: [PATCH 01/10] Delete functionality for Trie and TrieMap --- src/AssocList.mo | 35 ++++++++++ src/Trie.mo | 61 ++++++++++++++++-- src/TrieMap.mo | 4 ++ test/trieMapTest.mo | 152 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 246 insertions(+), 6 deletions(-) diff --git a/src/AssocList.mo b/src/AssocList.mo index fe50cd43..f8ad09e7 100644 --- a/src/AssocList.mo +++ b/src/AssocList.mo @@ -120,6 +120,41 @@ module { rec(map) }; + /// Remove the entry with `key` in `map`, by returning the reduced list. + /// Also returns the value of the removed entry if it existed and `null` otherwise. + /// Compares keys using the provided function `equal`. + /// + /// Example: + /// ```motoko include=import,initialize + /// // Add three entries to the map + /// // map = [(0, 10), (1, 11), (2, 12)] + /// map := AssocList.remove(map, 1, Nat.equal).0; + /// List.toArray(map) + /// ``` + /// Runtime: O(size) + /// + /// Space: O(size) + /// + /// *Runtime and space assumes that `equal` runs in O(1) time and space. + public func remove( + map : AssocList, + key : K, + equal : (K, K) -> Bool + ) : (AssocList, ?V) { + func rec(al : AssocList) : (AssocList, ?V) { + switch (al) { + case (null) { (null, null) }; + case (?((hd_k, hd_v), tl)) { + if (equal(key, hd_k)) { (tl, ?hd_v) } else { + let (tl2, value) = rec(tl); + (?((hd_k, hd_v), tl2), value) + } + } + } + }; + rec(map) + }; + /// Produces a new map containing all entries from `map1` whose keys are not /// contained in `map2`. The "extra" entries in `map2` are ignored. Compares /// keys using the provided function `equal`. diff --git a/src/Trie.mo b/src/Trie.mo index e4831228..16f049aa 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -252,7 +252,6 @@ module { /// ``` public func empty() : Trie { #empty }; - /// Get the size in O(1) time. /// /// For a more detailed overview of how to use a `Trie`, @@ -577,7 +576,7 @@ module { switch (x, y) { case (null, ?v) { v }; case (?v, null) { v }; - case (_, _) { Debug.trap "Trie.mergeDisjoint"} + case (_, _) { Debug.trap "Trie.mergeDisjoint" } } } ), @@ -1508,7 +1507,12 @@ module { updated_outer }; - /// Remove the given key's value in the trie; return the new trie + /// Remove the entry for the given key from the trie, by returning the reduced trie. + /// Also returns the removed value if the key existed and `null` otherwise. + /// Compares keys using the provided function `equal`. + /// + /// Note: The removal of an existing key actually shrinks the trie. + /// Thereby, the trie is reorganized by possibly recursively collapsing sibling nodes. /// /// For a more detailed overview of how to use a `Trie`, /// see the [User's Overview](#overview). @@ -1517,12 +1521,57 @@ module { /// ```motoko include=initialize /// trie := Trie.put(trie, key "hello", Text.equal, 42).0; /// trie := Trie.put(trie, key "bye", Text.equal, 32).0; - /// // remove the value associated with "hello" + /// // remove the entry associated with "hello" /// trie := Trie.remove(trie, key "hello", Text.equal).0; /// assert (Trie.get(trie, key "hello", Text.equal) == null); /// ``` - public func remove(t : Trie, k : Key, k_eq : (K, K) -> Bool) : (Trie, ?V) { - replace(t, k, k_eq, null) + public func remove(trie : Trie, key : Key, equal : (K, K) -> Bool) : (Trie, ?V) { + func combine(left : Trie, right : Trie) : Trie { + switch ((left, right)) { + case (#leaf(leftLeaf), #empty) { + #leaf(leftLeaf) + }; + case (#empty, #leaf(rightLeaf)) { + #leaf(rightLeaf) + }; + case ((#leaf(leftLeaf), #leaf(rightLeaf))) { + let size = leftLeaf.size + rightLeaf.size; + if (size <= MAX_LEAF_SIZE) { + let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); + #leaf({ size = size; keyvals = union }) + } else { + branch((#leaf(leftLeaf), #leaf(rightLeaf))) + } + }; + case ((left, right)) { + branch(left, right) + } + } + }; + + func rec(trie : Trie, bitPosition : Nat) : (Trie, ?V) { + switch trie { + case (#empty) { (#empty, null) }; + case (#leaf(oldLeaf)) { + let (newList, value) = AssocList.remove(oldLeaf.keyvals, key, equalKey(equal)); + (leaf(newList, bitPosition), value) + }; + case (#branch(oldBranch)) { + let bit = Hash.bit(key.hash, bitPosition); + if (not bit) { + let (newLeft, value) = rec(oldBranch.left, bitPosition + 1); + (combine(newLeft, oldBranch.right), value) + } else { + let (newRight, value) = rec(oldBranch.right, bitPosition + 1); + (combine(oldBranch.left, newRight), value) + } + } + } + }; + + let (newTrie, value) = rec(trie, 0); + //assert (isValid(newTrie, false)); + (newTrie, value) }; /// Remove the given key's value in the trie, diff --git a/src/TrieMap.mo b/src/TrieMap.mo index fd623f8e..29b00ce3 100644 --- a/src/TrieMap.mo +++ b/src/TrieMap.mo @@ -104,6 +104,8 @@ module { /// Delete the entry associated with key `key`, if it exists. If the key is /// absent, there is no effect. /// + /// Note: The deletion of an existing key shrinks the trie map. + /// /// Example: /// ```motoko include=initialize /// map.put(0, 10); @@ -121,6 +123,8 @@ module { /// Delete the entry associated with key `key`. Return the deleted value /// as an option if it exists, and `null` otherwise. /// + /// Note: The deletion of an existing key shrinks the trie map. + /// /// Example: /// ```motoko include=initialize /// map.put(0, 10); diff --git a/test/trieMapTest.mo b/test/trieMapTest.mo index eaa245be..f2bb0f02 100644 --- a/test/trieMapTest.mo +++ b/test/trieMapTest.mo @@ -4,6 +4,8 @@ import Iter "mo:base/Iter"; import Hash "mo:base/Hash"; import Text "mo:base/Text"; import Nat "mo:base/Nat"; +import Array "mo:base/Array"; +import Order "mo:base/Order"; import Suite "mo:matchers/Suite"; import T "mo:matchers/Testable"; @@ -356,6 +358,156 @@ let suite = Suite.suite( Suite.run(suite); +/* --------------------------------------- */ + +object Random { + var number = 4711; + public func next() : Nat { + number := (123138118391 * number + 133489131) % 9999; + number + } +}; + +func shuffle(array : [Nat]) : [Nat] { + let extended = Array.map(array, func(value) { (value, Random.next()) }); + let sorted = Array.sort<(Nat, Nat)>( + extended, + func(first, second) { + Nat.compare(first.1, second.1) + } + ); + Array.map<(Nat, Nat), Nat>( + sorted, + func(value) { + value.0 + } + ) +}; + +let testSize = 1_000; + +let testKeys = shuffle(Array.tabulate(testSize, func(index) { index })); + +func buildTestTrie() : TrieMap.TrieMap { + let trie = TrieMap.TrieMap(Nat.equal, Hash.hash); + for (key in testKeys.vals()) { + trie.put(key, debug_show (key)) + }; + trie +}; + +func expectedKeyValuePairs(keys : [Nat]) : [(Nat, Text)] { + Array.tabulate<(Nat, Text)>(keys.size(), func(index) { (keys[index], debug_show (keys[index])) }) +}; + +let expectedEntries = expectedKeyValuePairs(Array.sort(testKeys, Nat.compare)); +let expectedKeys = Array.sort(testKeys, Nat.compare); +let expectedValues = Array.sort(Array.map(expectedKeys, func(key) { debug_show (key) }), Text.compare); + +let entryTestable = T.tuple2Testable(T.natTestable, T.textTestable); + +func compareByKey(first : (Nat, Text), second : (Nat, Text)) : Order.Order { + Nat.compare(first.0, second.0) +}; + +func sortedEntries(trie : TrieMap.TrieMap) : [(Nat, Text)] { + Array.sort(Iter.toArray(trie.entries()), compareByKey) +}; + +class TrieMatcher(expected : [(Nat, Text)]) : M.Matcher> { + public func describeMismatch(actual : TrieMap.TrieMap, description : M.Description) { + Prim.debugPrint(debug_show (sortedEntries(actual)) # " should be " # debug_show (expected)) + }; + + public func matches(actual : TrieMap.TrieMap) : Bool { + sortedEntries(actual) == expected + } +}; + +let randomTestSuite = Suite.suite( + "random trie", + [ + Suite.test( + "size", + buildTestTrie().size(), + M.equals(T.nat(testSize)) + ), + Suite.test( + "iterate entries", + sortedEntries(buildTestTrie()), + M.equals(T.array<(Nat, Text)>(entryTestable, expectedEntries)) + ), + Suite.test( + "iterate keys", + Array.sort(Iter.toArray(buildTestTrie().keys()), Nat.compare), + M.equals(T.array(T.natTestable, expectedKeys)) + ), + Suite.test( + "iterate values", + Array.sort(Iter.toArray(buildTestTrie().vals()), Text.compare), + M.equals(T.array(T.textTestable, expectedValues)) + ), + Suite.test( + "get all", + do { + let trie = buildTestTrie(); + for (key in testKeys.vals()) { + let value = trie.get(key); + assert (value == ?debug_show (key)) + }; + trie + }, + TrieMatcher(expectedEntries) + ), + Suite.test( + "replace all", + do { + let trie = buildTestTrie(); + for (key in testKeys.vals()) { + let value = trie.replace(key, "TEST-" # debug_show (key)); + assert (value == ?debug_show (key)) + }; + trie + }, + TrieMatcher(Array.map(expectedKeys, func(key) { (key, "TEST-" # debug_show (key)) })) + ), + Suite.test( + "remove randomized", + do { + let trie = buildTestTrie(); + var count = 0; + for (key in testKeys.vals()) { + if (Random.next() % 2 == 0) { + let result = trie.remove(key); + assert (result == ?debug_show (key)); + count += 1 + } + }; + trie.size() == +testKeys.size() - count + }, + M.equals(T.bool(true)) + ), + Suite.test( + "clear", + do { + let trie = buildTestTrie(); + for ((key, value) in trie.entries()) { + // stable iteration + assert (debug_show (key) == value); + let result = trie.remove(key); + assert (result == ?debug_show (key)) + }; + trie + }, + TrieMatcher([]) + ) + ] +); + +Suite.run(randomTestSuite); + +/* --------------------------------------- */ + debug { let a = TrieMap.TrieMap(Text.equal, Text.hash); From 0f326e3d97f550f9c6509ce3be4dd8ef176f6a2d Mon Sep 17 00:00:00 2001 From: luc-blaeser Date: Tue, 14 Feb 2023 11:25:21 +0100 Subject: [PATCH 02/10] Code refactoring Extracting a helper function that does not access the closure. --- src/Trie.mo | 51 ++++++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/Trie.mo b/src/Trie.mo index 16f049aa..af32f5df 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -1507,6 +1507,30 @@ module { updated_outer }; + /// Combine two nodes that may have a reduced size after an entry deletion. + func combineReducedNodes(left : Trie, right : Trie) : Trie { + switch ((left, right)) { + case (#leaf(leftLeaf), #empty) { + #leaf(leftLeaf) + }; + case (#empty, #leaf(rightLeaf)) { + #leaf(rightLeaf) + }; + case ((#leaf(leftLeaf), #leaf(rightLeaf))) { + let size = leftLeaf.size + rightLeaf.size; + if (size <= MAX_LEAF_SIZE) { + let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); + #leaf({ size = size; keyvals = union }) + } else { + branch((#leaf(leftLeaf), #leaf(rightLeaf))) + } + }; + case ((left, right)) { + branch(left, right) + } + } + }; + /// Remove the entry for the given key from the trie, by returning the reduced trie. /// Also returns the removed value if the key existed and `null` otherwise. /// Compares keys using the provided function `equal`. @@ -1526,29 +1550,6 @@ module { /// assert (Trie.get(trie, key "hello", Text.equal) == null); /// ``` public func remove(trie : Trie, key : Key, equal : (K, K) -> Bool) : (Trie, ?V) { - func combine(left : Trie, right : Trie) : Trie { - switch ((left, right)) { - case (#leaf(leftLeaf), #empty) { - #leaf(leftLeaf) - }; - case (#empty, #leaf(rightLeaf)) { - #leaf(rightLeaf) - }; - case ((#leaf(leftLeaf), #leaf(rightLeaf))) { - let size = leftLeaf.size + rightLeaf.size; - if (size <= MAX_LEAF_SIZE) { - let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); - #leaf({ size = size; keyvals = union }) - } else { - branch((#leaf(leftLeaf), #leaf(rightLeaf))) - } - }; - case ((left, right)) { - branch(left, right) - } - } - }; - func rec(trie : Trie, bitPosition : Nat) : (Trie, ?V) { switch trie { case (#empty) { (#empty, null) }; @@ -1560,10 +1561,10 @@ module { let bit = Hash.bit(key.hash, bitPosition); if (not bit) { let (newLeft, value) = rec(oldBranch.left, bitPosition + 1); - (combine(newLeft, oldBranch.right), value) + (combineReducedNodes(newLeft, oldBranch.right), value) } else { let (newRight, value) = rec(oldBranch.right, bitPosition + 1); - (combine(oldBranch.left, newRight), value) + (combineReducedNodes(oldBranch.left, newRight), value) } } } From 7b638c9df70e65a37476055d781764e047f6008d Mon Sep 17 00:00:00 2001 From: Luc Blaeser <112870813+luc-blaeser@users.noreply.github.com> Date: Tue, 14 Feb 2023 15:28:42 +0100 Subject: [PATCH 03/10] Update src/Trie.mo Co-authored-by: Claudio Russo --- src/Trie.mo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Trie.mo b/src/Trie.mo index af32f5df..5ace99ba 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -1522,7 +1522,7 @@ module { let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); #leaf({ size = size; keyvals = union }) } else { - branch((#leaf(leftLeaf), #leaf(rightLeaf))) + branch((left, right)) } }; case ((left, right)) { From dda937d9ab1f7c91f54acdd84b55f74a0f906a7f Mon Sep 17 00:00:00 2001 From: luc-blaeser Date: Tue, 14 Feb 2023 18:14:14 +0100 Subject: [PATCH 04/10] Simplifying implememtation As replace with null is deleting in AssocList --- src/AssocList.mo | 35 ----------------------------------- src/Trie.mo | 2 +- 2 files changed, 1 insertion(+), 36 deletions(-) diff --git a/src/AssocList.mo b/src/AssocList.mo index f8ad09e7..fe50cd43 100644 --- a/src/AssocList.mo +++ b/src/AssocList.mo @@ -120,41 +120,6 @@ module { rec(map) }; - /// Remove the entry with `key` in `map`, by returning the reduced list. - /// Also returns the value of the removed entry if it existed and `null` otherwise. - /// Compares keys using the provided function `equal`. - /// - /// Example: - /// ```motoko include=import,initialize - /// // Add three entries to the map - /// // map = [(0, 10), (1, 11), (2, 12)] - /// map := AssocList.remove(map, 1, Nat.equal).0; - /// List.toArray(map) - /// ``` - /// Runtime: O(size) - /// - /// Space: O(size) - /// - /// *Runtime and space assumes that `equal` runs in O(1) time and space. - public func remove( - map : AssocList, - key : K, - equal : (K, K) -> Bool - ) : (AssocList, ?V) { - func rec(al : AssocList) : (AssocList, ?V) { - switch (al) { - case (null) { (null, null) }; - case (?((hd_k, hd_v), tl)) { - if (equal(key, hd_k)) { (tl, ?hd_v) } else { - let (tl2, value) = rec(tl); - (?((hd_k, hd_v), tl2), value) - } - } - } - }; - rec(map) - }; - /// Produces a new map containing all entries from `map1` whose keys are not /// contained in `map2`. The "extra" entries in `map2` are ignored. Compares /// keys using the provided function `equal`. diff --git a/src/Trie.mo b/src/Trie.mo index 5ace99ba..85dcf0ed 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -1554,7 +1554,7 @@ module { switch trie { case (#empty) { (#empty, null) }; case (#leaf(oldLeaf)) { - let (newList, value) = AssocList.remove(oldLeaf.keyvals, key, equalKey(equal)); + let (newList, value) = AssocList.replace(oldLeaf.keyvals, key, equalKey(equal), null); (leaf(newList, bitPosition), value) }; case (#branch(oldBranch)) { From 550e675890890b1c26ca2c3e54f897578ee68892 Mon Sep 17 00:00:00 2001 From: luc-blaeser Date: Wed, 15 Feb 2023 12:10:15 +0100 Subject: [PATCH 05/10] Also collapse two empty sibling nodes --- src/Trie.mo | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Trie.mo b/src/Trie.mo index 85dcf0ed..58c2cdb4 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -1510,6 +1510,9 @@ module { /// Combine two nodes that may have a reduced size after an entry deletion. func combineReducedNodes(left : Trie, right : Trie) : Trie { switch ((left, right)) { + case (#empty, #empty) { + #empty + }; case (#leaf(leftLeaf), #empty) { #leaf(leftLeaf) }; From 96c58ae41d5c0e5b26d8c045912f82cf69cae42e Mon Sep 17 00:00:00 2001 From: luc-blaeser Date: Mon, 20 Feb 2023 13:53:12 +0100 Subject: [PATCH 06/10] Apply reduction to other functions --- src/Trie.mo | 117 ++++++++++++++++++++++------------------------------ 1 file changed, 50 insertions(+), 67 deletions(-) diff --git a/src/Trie.mo b/src/Trie.mo index 58c2cdb4..a6bdd5df 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -353,7 +353,48 @@ module { /// Purely-functional representation permits _O(1)_ copy, via persistent sharing. public func clone(t : Trie) : Trie = t; - /// Replace the given key's value option with the given one, returning the previous one + /// Combine two nodes that may have a reduced size after an entry deletion. + func combineReducedNodes(left : Trie, right : Trie) : Trie { + switch ((left, right)) { + case (#empty, #empty) { + #empty + }; + case (#leaf(leftLeaf), #empty) { + #leaf(leftLeaf) + }; + case (#empty, #leaf(rightLeaf)) { + #leaf(rightLeaf) + }; + case ((#leaf(leftLeaf), #leaf(rightLeaf))) { + let size = leftLeaf.size + rightLeaf.size; + if (size <= MAX_LEAF_SIZE) { + let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); + #leaf({ size = size; keyvals = union }) + } else { + branch((left, right)) + } + }; + case ((left, right)) { + branch(left, right) + } + } + }; + + /// Replace the given key's value option with the given value, returning the modified trie. + /// Also returns the replaced value if the key existed and `null` otherwise. + /// Compares keys using the provided function `k_eq`. + /// + /// Note: Replacing a key's value by `null` removes the key and also shrinks the trie. + /// + /// For a more detailed overview of how to use a `Trie`, + /// see the [User's Overview](#overview). + /// + /// Example: + /// ```motoko include=initialize + /// trie := Trie.put(trie, key "test", Text.equal, 1).0; + /// trie := Trie.replace(trie, key "test", Text.equal, 42).0; + /// assert (Trie.get(trie, key "hello", Text.equal) == ?42); + /// ``` public func replace(t : Trie, k : Key, k_eq : (K, K) -> Bool, v : ?V) : (Trie, ?V) { let key_eq = equalKey(k_eq); @@ -368,10 +409,10 @@ module { // rebuild either the left or right path with the (k, v) pair if (not bit) { let (l, v_) = rec(b.left, bitpos + 1); - (branch(l, b.right), v_) + (combineReducedNodes(l, b.right), v_) } else { let (r, v_) = rec(b.right, bitpos + 1); - (branch(b.left, r), v_) + (combineReducedNodes(b.left, r), v_) } }; case (#leaf(l)) { @@ -1286,11 +1327,7 @@ module { case (#branch(b)) { let fl = rec(b.left, bitpos + 1); let fr = rec(b.right, bitpos + 1); - if (isEmpty(fl) and isEmpty(fr)) { - #empty - } else { - branch(fl, fr) - } + combineReducedNodes(fl, fr) } } }; @@ -1338,11 +1375,7 @@ module { case (#branch(b)) { let fl = rec(b.left, bitpos + 1); let fr = rec(b.right, bitpos + 1); - if (isEmpty(fl) and isEmpty(fr)) { - #empty - } else { - branch(fl, fr) - } + combineReducedNodes(fl, fr) } } }; @@ -1507,39 +1540,11 @@ module { updated_outer }; - /// Combine two nodes that may have a reduced size after an entry deletion. - func combineReducedNodes(left : Trie, right : Trie) : Trie { - switch ((left, right)) { - case (#empty, #empty) { - #empty - }; - case (#leaf(leftLeaf), #empty) { - #leaf(leftLeaf) - }; - case (#empty, #leaf(rightLeaf)) { - #leaf(rightLeaf) - }; - case ((#leaf(leftLeaf), #leaf(rightLeaf))) { - let size = leftLeaf.size + rightLeaf.size; - if (size <= MAX_LEAF_SIZE) { - let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); - #leaf({ size = size; keyvals = union }) - } else { - branch((left, right)) - } - }; - case ((left, right)) { - branch(left, right) - } - } - }; - /// Remove the entry for the given key from the trie, by returning the reduced trie. /// Also returns the removed value if the key existed and `null` otherwise. - /// Compares keys using the provided function `equal`. + /// Compares keys using the provided function `k_eq`. /// - /// Note: The removal of an existing key actually shrinks the trie. - /// Thereby, the trie is reorganized by possibly recursively collapsing sibling nodes. + /// Note: The removal of an existing key shrinks the trie. /// /// For a more detailed overview of how to use a `Trie`, /// see the [User's Overview](#overview). @@ -1552,30 +1557,8 @@ module { /// trie := Trie.remove(trie, key "hello", Text.equal).0; /// assert (Trie.get(trie, key "hello", Text.equal) == null); /// ``` - public func remove(trie : Trie, key : Key, equal : (K, K) -> Bool) : (Trie, ?V) { - func rec(trie : Trie, bitPosition : Nat) : (Trie, ?V) { - switch trie { - case (#empty) { (#empty, null) }; - case (#leaf(oldLeaf)) { - let (newList, value) = AssocList.replace(oldLeaf.keyvals, key, equalKey(equal), null); - (leaf(newList, bitPosition), value) - }; - case (#branch(oldBranch)) { - let bit = Hash.bit(key.hash, bitPosition); - if (not bit) { - let (newLeft, value) = rec(oldBranch.left, bitPosition + 1); - (combineReducedNodes(newLeft, oldBranch.right), value) - } else { - let (newRight, value) = rec(oldBranch.right, bitPosition + 1); - (combineReducedNodes(oldBranch.left, newRight), value) - } - } - } - }; - - let (newTrie, value) = rec(trie, 0); - //assert (isValid(newTrie, false)); - (newTrie, value) + public func remove(t : Trie, k : Key, k_eq : (K, K) -> Bool) : (Trie, ?V) { + replace(t, k, k_eq, null) }; /// Remove the given key's value in the trie, From 2d6e0c353de5406aff1a743db8b45603a8a902fa Mon Sep 17 00:00:00 2001 From: luc-blaeser Date: Tue, 28 Feb 2023 08:55:31 +0100 Subject: [PATCH 07/10] Optimize replace function (second return value) --- src/Trie.mo | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Trie.mo b/src/Trie.mo index a6bdd5df..39edbd77 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -397,33 +397,36 @@ module { /// ``` public func replace(t : Trie, k : Key, k_eq : (K, K) -> Bool, v : ?V) : (Trie, ?V) { let key_eq = equalKey(k_eq); + var replacedValue: ?V = null; - func rec(t : Trie, bitpos : Nat) : (Trie, ?V) { + func recursiveReplace(t : Trie, bitpos : Nat) : Trie { switch t { case (#empty) { let (kvs, _) = AssocList.replace(null, k, key_eq, v); - (leaf(kvs, bitpos), null) + replacedValue := null; + leaf(kvs, bitpos) }; case (#branch(b)) { let bit = Hash.bit(k.hash, bitpos); // rebuild either the left or right path with the (k, v) pair if (not bit) { - let (l, v_) = rec(b.left, bitpos + 1); - (combineReducedNodes(l, b.right), v_) + let l = recursiveReplace(b.left, bitpos + 1); + combineReducedNodes(l, b.right) } else { - let (r, v_) = rec(b.right, bitpos + 1); - (combineReducedNodes(b.left, r), v_) + let r = recursiveReplace(b.right, bitpos + 1); + combineReducedNodes(b.left, r) } }; case (#leaf(l)) { - let (kvs2, old_val) = AssocList.replace(l.keyvals, k, key_eq, v); - (leaf(kvs2, bitpos), old_val) + let (kvs2, oldValue) = AssocList.replace(l.keyvals, k, key_eq, v); + replacedValue := oldValue; + leaf(kvs2, bitpos) } } }; - let (to, vo) = rec(t, 0); - //assert(isValid(to, false)); - (to, vo) + let newTrie = recursiveReplace(t, 0); + //assert(isValid(newTrie, false)); + (newTrie, replacedValue) }; /// Put the given key's value in the trie; return the new trie, and the previous value associated with the key, if any. From bf9e3cf9a103ae01d45418ba16c9e867df1b8c1b Mon Sep 17 00:00:00 2001 From: Claudio Russo Date: Tue, 28 Feb 2023 08:21:28 +0000 Subject: [PATCH 08/10] parens removal --- src/Trie.mo | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Trie.mo b/src/Trie.mo index a6bdd5df..9a8bac44 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -355,17 +355,17 @@ module { /// Combine two nodes that may have a reduced size after an entry deletion. func combineReducedNodes(left : Trie, right : Trie) : Trie { - switch ((left, right)) { + switch (left, right) { case (#empty, #empty) { #empty }; - case (#leaf(leftLeaf), #empty) { + case (#leaf leftLeaf, #empty) { #leaf(leftLeaf) }; - case (#empty, #leaf(rightLeaf)) { + case (#empty, #leaf rightLeaf) { #leaf(rightLeaf) }; - case ((#leaf(leftLeaf), #leaf(rightLeaf))) { + case (#leaf leftLeaf, #leaf rightLeaf) { let size = leftLeaf.size + rightLeaf.size; if (size <= MAX_LEAF_SIZE) { let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); @@ -374,7 +374,7 @@ module { branch((left, right)) } }; - case ((left, right)) { + case (left, right) { branch(left, right) } } From fb0ffb307462a4a03a7033f2e245ceacc7c86334 Mon Sep 17 00:00:00 2001 From: Claudio Russo Date: Tue, 28 Feb 2023 08:26:13 +0000 Subject: [PATCH 09/10] remove parens --- src/Trie.mo | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Trie.mo b/src/Trie.mo index 400df19c..301bc35a 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -359,19 +359,19 @@ module { case (#empty, #empty) { #empty }; - case (#leaf leftLeaf, #empty) { + case (#leaf(leftLeaf), #empty) { #leaf(leftLeaf) }; - case (#empty, #leaf rightLeaf) { + case (#empty, #leaf(rightLeaf)) { #leaf(rightLeaf) }; - case (#leaf leftLeaf, #leaf rightLeaf) { + case (#leaf(leftLeaf), #leaf(rightLeaf)) { let size = leftLeaf.size + rightLeaf.size; if (size <= MAX_LEAF_SIZE) { let union = List.append(leftLeaf.keyvals, rightLeaf.keyvals); #leaf({ size = size; keyvals = union }) } else { - branch((left, right)) + branch(left, right) } }; case (left, right) { From 76ebaafc082732661c6d9427711fc7d20919e359 Mon Sep 17 00:00:00 2001 From: Luc Blaeser <112870813+luc-blaeser@users.noreply.github.com> Date: Tue, 28 Feb 2023 11:51:15 +0100 Subject: [PATCH 10/10] Remove redundant null assignment Co-authored-by: Claudio Russo --- src/Trie.mo | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Trie.mo b/src/Trie.mo index 301bc35a..a8793db5 100644 --- a/src/Trie.mo +++ b/src/Trie.mo @@ -403,7 +403,6 @@ module { switch t { case (#empty) { let (kvs, _) = AssocList.replace(null, k, key_eq, v); - replacedValue := null; leaf(kvs, bitpos) }; case (#branch(b)) {