scala / scala-library-next

backwards-binary-compatible Scala standard library additions
Apache License 2.0
69 stars 17 forks source link

Consider a `getAndRemove` operation on `Map`s #86

Open benhutchison opened 3 years ago

benhutchison commented 3 years ago

AFAIK, there isnt an easy way to get and remove an entry from a Map in a single lookup. remove removes it but doesnt expose the removed value. The desired operation would return a pair of the updated map, and the removed value (if any) wrapped in an Option.

In performance-critical code, a double lookup is not acceptable for something that can be readily done in a single visit to the map.

The operation can be performed in a single pass using updatedWith but the resulting code is non-obvious and involves a transient var. It's not the sort of code clients of the API should need to be writing.

NthPortal commented 3 years ago

mutable.Map has remove(K):Option[V], even though the abstract method subtractOne(K):Map.this.type does not return the removed value.

I assume you're referring to immutable.Map, where the method removed(K):Map[K,V] returns a (potentially new) Map. for immutable.Map, such a method would have to return (Map[K,V],Option[V]), which is not a super fun return type. still doable though

benhutchison commented 3 years ago

Precedent: https://www.scala-lang.org/api/current/scala/collection/immutable/Queue.html#dequeueOption:Option[(A,scala.collection.immutable.Queue[A])]

NthPortal commented 3 years ago

it's worth noting that you can't gain efficiency from this change in scala-library-next. it doesn't have the ability to change implementations in the stdlib, so it would end up having to look up the value and then remove it separately anyway

benhutchison commented 3 years ago

As I mentioned above, it can be done in one pass using the existing updatedWith.

Here's an example (verbatim from my week3 solution of the Effective Scala course)

  def delete(id: Id): Boolean =
    var removed = Option.empty[Task]
    val tasks2 = loadTasks().toMap.updatedWith(id){
      case t@Some(task) => {removed = t; None}
      case None => None
    }
    saveTasks(Tasks(tasks2))
    removed.isDefined