implement Map::update

[juliano]

Fixes #1965

And improves Iter::group_by

[peter-jerry-ye-code-review]

Iter::group_by implementation could be simplified further

Category
Maintainability
Code Snippet
let _ = result.update(key, fn(arr) {
match arr {
Some(existing) => existing + [element]
None => [element]
}
})
Recommendation
The match expression could be simplified using array concatenation directly:

let _ = result.update(key, fn(arr) { 
  arr.unwrap_or([]) + [element]
})

Reasoning
The proposed change makes the code more concise while maintaining the same functionality. Using unwrap_or provides a cleaner way to handle the None case without explicit pattern matching.

Unnecessary assignment to discard value in Map::update tests

Category
Maintainability
Code Snippet
let _ = map.update("a", fn(v) {...})
Recommendation
Remove the let binding when the return value isn't needed:

map.update("a", fn(v) {...})

Reasoning
Using let _ = is unnecessary when we don't need to capture or use the return value. Removing it makes the code cleaner and more direct.

Inefficient array copying in Map::update test with different value types

Category
Performance
Code Snippet
Some(arr) => {
let new_arr = []
for i in arr {
new_arr.push(i)
}
new_arr.push(1)
new_arr
}
Recommendation
Use array concatenation instead of manual copying:

Some(arr) => arr + [1]

Reasoning
The current implementation manually copies array elements one by one, which is less efficient than using built-in array concatenation. The proposed change is both more performant and more readable.

[coveralls]

Pull Request Test Coverage Report for Build 6545

Details

• 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.003%) to 92.5%

---

Totals
Change from base Build 6539:	0.003%
Covered Lines:	6117
Relevant Lines:	6613

---

💛 - Coveralls

[peter-jerry-ye]

The goal was to be efficient, which is not the case with this implementation.

I think you can copy the code of set and modify the line curr_entry.value = value to curr_entry.value = f(Some(value)) and the let entry = { .., value, ..} to let entry = { .., value : f(None), .. }

[peter-jerry-ye]

Something like this:

  if self.size >= self.growAt {
    self.grow()
  }
  let hash = key.hash()
  let (idx, psl) = for psl = 0, idx = hash & self.capacity_mask {
    match self.entries[idx] {
      None => break (idx, psl)
      Some(curr_entry) => {
        if curr_entry.hash == hash && curr_entry.key == key {
          let new_value = f(Some(value))
          curr_entry.value = new_value
          return new_value
        }
        if psl > curr_entry.psl {
          self.push_away(idx, curr_entry)
          break (idx, psl)
        }
        continue psl + 1, (idx + 1) & self.capacity_mask
      }
    }
  }
  let new_value = f(None)
  let entry = { prev: self.tail, next: None, psl, key, value: new_value, hash }
  self.add_entry_to_tail(idx, entry)
  return new_value

[peter-jerry-ye]

I also have some doubt about the function name and function signature. In Rust it might corresponds to entry.and_modify.or_insert. In Java the closest thing I find is compute.

However the return value of compute is also V?, where None means deleteting the value, which I think we can also have something similar.

As of the name, I'm not quite sure whether it should be called compute or update. When updating a map, I tend to feel that we get the old value back, like replace in Java or insert in Rust; or self for chaining.

[hackwaly]

I also have some doubt about the function name and function signature

FYI: https://github.com/tc39/proposal-upsert

[peter-jerry-ye]

FYI: https://github.com/tc39/proposal-upsert

Thank you @hackwaly . But our operation is more complex than that, so maybe we need a name like compute_then_insert_or_update_then_get. Maybe it would make more sense if we simplify what this function does.