Skip to content

refactor(map): align op_get with array #2305

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

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
3 changes: 1 addition & 2 deletions builtin/builtin.mbti
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,7 @@ fn[K, V] Map::iter2(Self[K, V]) -> Iter2[K, V]
fn[K, V] Map::keys(Self[K, V]) -> Iter[K]
fn[K, V] Map::new(capacity~ : Int = ..) -> Self[K, V]
fn[K : Hash + Eq, V] Map::of(FixedArray[(K, V)]) -> Self[K, V]
#deprecated
fn[K : Hash + Eq, V] Map::op_get(Self[K, V], K) -> V?
fn[K : Hash + Eq, V] Map::op_get(Self[K, V], K) -> V
fn[K : Hash + Eq, V] Map::op_set(Self[K, V], K, V) -> Unit
fn[K : Hash + Eq, V] Map::remove(Self[K, V], K) -> Unit
fn[K : Hash + Eq, V] Map::set(Self[K, V], K, V) -> Unit
Expand Down
13 changes: 10 additions & 3 deletions builtin/linked_hash_map.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,16 @@ pub fn[K : Hash + Eq, V] Map::get(self : Map[K, V], key : K) -> V? {
}

///|
#deprecated("Use `get` instead. `op_get` will return `V` instead of `Option[V]` in the future.")
pub fn[K : Hash + Eq, V] Map::op_get(self : Map[K, V], key : K) -> V? {
self.get(key)
pub fn[K : Hash + Eq, V] Map::op_get(self : Map[K, V], key : K) -> V {
let hash = key.hash()
for i = 0, idx = hash & self.capacity_mask {
guard self.entries[idx] is Some(entry)
if entry.hash == hash && entry.key == key {
return entry.value
}
guard i <= entry.psl
continue i + 1, (idx + 1) & self.capacity_mask
}
}

///|
Expand Down
18 changes: 14 additions & 4 deletions builtin/linked_hash_map_wbtest.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ test "get" {
assert_eq(m.get("b"), Some(2))
assert_eq(m.get("c"), Some(3))
assert_eq(m.get("d"), None)

// pattern
guard m is { "a": 1, "b": 2, "c": 3, "d"? : None, .. }
}

///|
Expand Down Expand Up @@ -115,9 +118,16 @@ test "op_get" {
let m : Map[String, Int] = Map::new()
m.set("a", 1)
m.set("b", 2)
assert_eq(m["a"], Some(1))
assert_eq(m["b"], Some(2))
assert_eq(m["c"], None)
assert_eq(m["a"], 1)
assert_eq(m["b"], 2)
}

///|
test "panic op_get" {
let m : Map[String, Int] = Map::new()
m.set("a", 1)
m.set("b", 2)
m["c"] |> ignore
}

///|
Expand Down Expand Up @@ -734,7 +744,7 @@ test "hash collision" {
m.set("a", 1)
m.set("ab", 2)
m.set("abc", 3)
inspect(m["d"], content="None")
inspect(m.get("d"), content="None")
}

///|
Expand Down
6 changes: 0 additions & 6 deletions hashmap/deprecated.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,3 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

///|
#deprecated("Use `get` instead. `op_get` will return `V` instead of `Option[V]` in the future.")
pub fn[K : Hash + Eq, V] op_get(self : T[K, V], key : K) -> V? {
self.get(key)
}
28 changes: 28 additions & 0 deletions hashmap/hashmap.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,34 @@ pub fn[K : Hash + Eq, V] get(self : T[K, V], key : K) -> V? {
}
}

///|
/// Retrieves the value associated with a given key in the hash map.
///
/// Parameters:
///
/// * `self` : The hash map to search in.
/// * `key` : The key to look up in the map.
///
/// Returns `value` if the key exists in the map, panic otherwise.
///
/// Example:
///
/// ```moonbit
/// let map = @hashmap.of([("key", 42)])
/// inspect(map["key"], content="42")
/// ```
pub fn[K : Hash + Eq, V] op_get(self : T[K, V], key : K) -> V {
let hash = key.hash()
for i = 0, idx = hash & self.capacity_mask {
guard self.entries[idx] is Some(entry)
if entry.hash == hash && entry.key == key {
break entry.value
}
guard entry.psl <= i
continue i + 1, (idx + 1) & self.capacity_mask
}
}

///|
fn[K : Hash + Eq, V] get_with_hash(self : T[K, V], key : K, hash : Int) -> V? {
for i = 0, idx = hash & self.capacity_mask {
Expand Down
3 changes: 1 addition & 2 deletions hashmap/hashmap.mbti
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ fn[K : Hash + Eq, V] T::get_or_init(Self[K, V], K, () -> V) -> V
fn[K, V] T::is_empty(Self[K, V]) -> Bool
fn[K, V] T::iter(Self[K, V]) -> Iter[(K, V)]
fn[K, V] T::iter2(Self[K, V]) -> Iter2[K, V]
#deprecated
fn[K : Hash + Eq, V] T::op_get(Self[K, V], K) -> V?
fn[K : Hash + Eq, V] T::op_get(Self[K, V], K) -> V
fn[K : Hash + Eq, V] T::op_set(Self[K, V], K, V) -> Unit
fn[K : Hash + Eq, V] T::remove(Self[K, V], K) -> Unit
fn[K : Hash + Eq, V] T::set(Self[K, V], K, V) -> Unit
Expand Down
16 changes: 13 additions & 3 deletions hashmap/hashmap_test.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ test "get" {
inspect(m.get("b"), content="Some(2)")
inspect(m.get("c"), content="Some(3)")
inspect(m.get("d"), content="None")

// pattern
guard m is { "a": 1, "b": 2, "c": 3, "d"? : None, .. }
}

///|
Expand Down Expand Up @@ -57,9 +60,16 @@ test "op_get" {
let m = @hashmap.new()
m.set("a", 1)
m.set("b", 2)
assert_eq(m.get("a"), Some(1))
assert_eq(m.get("b"), Some(2))
assert_eq(m.get("c"), None)
assert_eq(m["a"], 1)
assert_eq(m["b"], 2)
}

///|
test "panic op_get" {
let m = @hashmap.new()
m.set("a", 1)
m.set("b", 2)
m["c"] |> ignore
}

///|
Expand Down
3 changes: 1 addition & 2 deletions immut/sorted_map/sorted_map.mbti
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ fn[K, X, Y] T::map_with_key(Self[K, X], (K, X) -> Y) -> Self[K, Y]
fn[K, V] T::new() -> Self[K, V]
#deprecated
fn[K : Compare, V] T::of(FixedArray[(K, V)]) -> Self[K, V]
#deprecated
fn[K : Compare, V] T::op_get(Self[K, V], K) -> V?
fn[K : Compare, V] T::op_get(Self[K, V], K) -> V
fn[K : Compare, V] T::remove(Self[K, V], K) -> Self[K, V]
#deprecated
fn[K, V] T::singleton(K, V) -> Self[K, V]
Expand Down
19 changes: 16 additions & 3 deletions immut/sorted_map/utils.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,22 @@ pub fn[K : Compare, V] get(self : T[K, V], key : K) -> V? {
}

///|
#deprecated("Use `get` instead. `op_get` will return `V` instead of `Option[V]` in the future.")
pub fn[K : Compare, V] op_get(self : T[K, V], key : K) -> V? {
self.get(key)
/// Get the value associated with a key.
/// O(log n).
pub fn[K : Compare, V] op_get(self : T[K, V], key : K) -> V {
loop self {
Empty => panic()
Tree(k, value~, l, r, ..) => {
let c = key.compare(k)
if c == 0 {
value
} else if c < 0 {
continue l
} else {
continue r
}
}
}
}

///|
Expand Down
79 changes: 74 additions & 5 deletions immut/sorted_map/utils_test.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// limitations under the License.

///|
test "op_get with existing key" {
test "get with existing key" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
Expand All @@ -25,7 +25,7 @@ test "op_get with existing key" {
}

///|
test "op_get with non-existing key" {
test "get with non-existing key" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
Expand All @@ -37,7 +37,20 @@ test "op_get with non-existing key" {
}

///|
test "op_get after insertion" {
test "get with pattern" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
(1, "one"),
(2, "two"),
(0, "zero"),
])
guard map
is { 3: "three", 8: "eight", 1: "one", 2: "two", 0: "zero", 4? : None, .. }
}

///|
test "get after insertion" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
Expand All @@ -50,7 +63,7 @@ test "op_get after insertion" {
}

///|
test "op_get after removal" {
test "get after removal" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
Expand All @@ -62,6 +75,56 @@ test "op_get after removal" {
assert_eq(map.get(3), None)
}

///|
test "op_get with existing key" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
(1, "one"),
(2, "two"),
(0, "zero"),
])
assert_eq(map[3], "three")
}

///|
test "panic op_get with non-existing key" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
(1, "one"),
(2, "two"),
(0, "zero"),
])
map[4] |> ignore
}

///|
test "op_get after insertion" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
(1, "one"),
(2, "two"),
(0, "zero"),
])
let map = map.add(4, "four")
assert_eq(map[4], "four")
}

///|
test "panic op_get after removal" {
let map = @sorted_map.of([
(3, "three"),
(8, "eight"),
(1, "one"),
(2, "two"),
(0, "zero"),
])
let map = map.remove(3)
map[3] |> ignore
}

///|
test "iter" {
let map = [(3, "three"), (8, "eight"), (1, "one"), (2, "two"), (0, "zero")]
Expand Down Expand Up @@ -96,11 +159,17 @@ test "iter" {
}

///|
test "op_get with empty map" {
test "get with empty map" {
let map : @sorted_map.T[Int, String] = @sorted_map.new()
assert_eq(map.get(3), None)
}

///|
test "panic op_get with empty map" {
let map : @sorted_map.T[Int, String] = @sorted_map.new()
map[3] |> ignore
}

///|
test "to_json with non-empty map" {
let map = [(3, "three"), (8, "eight"), (1, "one"), (2, "two"), (0, "zero")]
Expand Down
6 changes: 0 additions & 6 deletions sorted_map/deprecated.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

///|
#deprecated("Use `get` instead. `op_get` will return `V` instead of `Option[V]` in the future.")
pub fn[K : Compare, V] op_get(self : T[K, V], key : K) -> V? {
self.get(key)
}

///|
#deprecated("Use @sorted_map.from_array instead")
pub fn[K : Compare, V] of(entries : Array[(K, V)]) -> T[K, V] {
Expand Down
18 changes: 18 additions & 0 deletions sorted_map/map.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,24 @@ pub fn[K : Compare, V] get(self : T[K, V], key : K) -> V? {
}
}

///|
/// Gets a value by a key.
pub fn[K : Compare, V] op_get(self : T[K, V], key : K) -> V {
loop self.root {
Some(node) => {
let cmp = key.compare(node.key)
if cmp == 0 {
break node.value
} else if cmp > 0 {
continue node.right
} else {
continue node.left
}
}
None => panic()
}
}

///|
/// Checks if map contains a key-value pair.
pub fn[K : Compare, V] contains(self : T[K, V], key : K) -> Bool {
Expand Down
31 changes: 12 additions & 19 deletions sorted_map/map_test.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ test "get" {
,
)
inspect(map.get(4), content="None")

// pattern
guard map is { 1: "a", 2: "b", 3: "c", 4? : None, .. }
}

///|
Expand All @@ -72,25 +75,15 @@ test "contains" {
///|
test "op_get" {
let map = @sorted_map.from_array([(3, "c"), (2, "b"), (1, "a")])
inspect(
map.get(1),
content=
#|Some("a")
,
)
inspect(
map.get(2),
content=
#|Some("b")
,
)
inspect(
map.get(3),
content=
#|Some("c")
,
)
inspect(map.get(4), content="None")
inspect(map[1], content="a")
inspect(map[2], content="b")
inspect(map[3], content="c")
}

///|
test "panic op_get" {
let map = @sorted_map.from_array([(3, "c"), (2, "b"), (1, "a")])
map[4] |> ignore
}

///|
Expand Down
Loading