Following a suggestion of jrudolph, made filter...

Following a suggestion of jrudolph, made filterKeys and mapValues
transform abstract maps, and duplicated functionality for immutable
maps. Moved transform and filterNot from immutable to general maps.
Review by phaller.

4 files changed, 57 insertions, 26 deletions

diff --git a/src/library/scala/collection/MapLike.scala b/src/library/scala/collection/MapLike.scala
index 0a00abeb47..7558f1eece 100644
--- a/src/library/scala/collection/MapLike.scala
+++ b/src/library/scala/collection/MapLike.scala
@@ -230,7 +230,7 @@ self =>
    *  @return an immutable map consisting only of those key value pairs of this map where the key satisfies
    *          the predicate `p`. The resulting map wraps the original map without copying any elements.
    */
-  def filterKeys(p: A => Boolean) = new DefaultMap[A, B] {
+  def filterKeys(p: A => Boolean): Map[A, B] = new DefaultMap[A, B] {
     override def foreach[C](f: ((A, B)) => C): Unit = for (kv <- self) if (p(kv._1)) f(kv)
     def iterator = self.iterator.filter(kv => p(kv._1))
     override def contains(key: A) = self.contains(key) && p(key)
@@ -245,7 +245,7 @@ self =>
   /** A map view resulting from applying a given function `f` to each value
    *  associated with a key in this map.
    */
-  def mapValues[C](f: B => C) = new DefaultMap[A, C] {
+  def mapValues[C](f: B => C): Map[A, C] = new DefaultMap[A, C] {
     override def foreach[D](g: ((A, C)) => D): Unit = for ((k, v) <- self) g((k, f(v)))
     def iterator = for ((k, v) <- self.iterator) yield (k, f(v))
     override def size = self.size
@@ -255,6 +255,35 @@ self =>
 
   @deprecated("use `mapValues' instead") def mapElements[C](f: B => C) = mapValues(f)
 
+  /** This function transforms all the values of mappings contained
+   *  in this map with function <code>f</code>.
+   *
+   *  @param f A function over keys and values
+   *  @return  the updated map
+   */
+  def transform[C, That](f: (A, B) => C)(implicit bf: CanBuildFrom[This, (A, C), That]): That = {
+    val b = bf(repr)
+    for ((key, value) <- this) b += ((key, f(key, value)))
+    b.result
+  }
+
+  /** Returns a new map with all key/value pairs for which the predicate
+   *  <code>p</code> returns <code>true</code>.
+   *
+   *  @param p A predicate over key-value pairs
+   *  @note    This method works by successively removing elements fro which the
+   *           predicate is false from this set.
+   *           If removal is slow, or you expect that most elements of the set$
+   *           will be removed, you might consider using <code>filter</code>
+   *           with a negated predicate instead.
+   */
+  override def filterNot(p: ((A, B)) => Boolean): This = {
+    var res: This = repr
+    for (kv <- this)
+      if (p(kv)) res = (res - kv._1).asInstanceOf[This] // !!! concrete overrides abstract problem
+    res
+  }
+
   // The following 5 operations (updated, two times +, two times ++) should really be
   // generic, returning This[B]. We need better covariance support to express that though.
   // So right now we do the brute force approach of code duplication.
diff --git a/src/library/scala/collection/TraversableLike.scala b/src/library/scala/collection/TraversableLike.scala
index 78890ca406..2752f29c00 100644
--- a/src/library/scala/collection/TraversableLike.scala
+++ b/src/library/scala/collection/TraversableLike.scala
@@ -676,7 +676,7 @@ self =>
    *  @tparam   B     The type over which the ordering is defined.
    *  @return   the largest element of this $coll with respect to the ordering `cmp`.
    *
-   *  @usecase def min: A
+   *  @usecase def max: A
    *  @return   the largest element of this $coll.
    */
   def max[B >: A](implicit cmp: Ordering[B]): A = {
diff --git a/src/library/scala/collection/immutable/MapLike.scala b/src/library/scala/collection/immutable/MapLike.scala
index a06bce1038..88f938fb56 100644
--- a/src/library/scala/collection/immutable/MapLike.scala
+++ b/src/library/scala/collection/immutable/MapLike.scala
@@ -85,33 +85,32 @@ self =>
   override def ++[B1 >: B] (iter: Iterator[(A, B1)]): immutable.Map[A, B1] =
     ((repr: immutable.Map[A, B1]) /: iter) (_ + _)
 
-  /** This function transforms all the values of mappings contained
-   *  in this map with function <code>f</code>.
-   *
-   *  @param f A function over keys and values
-   *  @return  the updated map
+  /** Filters this map by retaining only keys satisfying a predicate.
+   *  @param  p   the predicate used to test keys
+   *  @return an immutable map consisting only of those key value pairs of this map where the key satisfies
+   *          the predicate `p`. The resulting map wraps the original map without copying any elements.
    */
-  def transform[C, That](f: (A, B) => C)(implicit bf: CanBuildFrom[This, (A, C), That]): That = {
-    val b = bf(repr)
-    for ((key, value) <- this) b += ((key, f(key, value)))
-    b.result
+  override def filterKeys(p: A => Boolean): Map[A, B] = new DefaultMap[A, B] {
+    override def foreach[C](f: ((A, B)) => C): Unit = for (kv <- self) if (p(kv._1)) f(kv)
+    def iterator = self.iterator.filter(kv => p(kv._1))
+    override def contains(key: A) = self.contains(key) && p(key)
+    def get(key: A) = if (!p(key)) None else self.get(key)
   }
 
-  /** Returns a new map with all key/value pairs for which the predicate
-   *  <code>p</code> returns <code>true</code>.
-   *
-   *  @param p A predicate over key-value pairs
-   *  @note    This method works by successively removing elements fro which the
-   *           predicate is false from this set.
-   *           If removal is slow, or you expect that most elements of the set$
-   *           will be removed, you might consider using <code>filter</code>
-   *           with a negated predicate instead.
+  /** Transforms this map by applying a function to every retrieved value.
+   *  @param  d   the function used to transform values of this map.
+   *  @return an immutable map which maps every key of this map
+   *          to `f(this(key))`. The resulting map wraps the original map without copying any elements.
+   */
+  /** A map view resulting from applying a given function `f` to each value
+   *  associated with a key in this map.
    */
-  override def filterNot(p: ((A, B)) => Boolean): This = {
-    var res: This = repr
-    for (kv <- this)
-      if (p(kv)) res = (res - kv._1).asInstanceOf[This] // !!! concrete overrides abstract problem
-    res
+  override def mapValues[C](f: B => C): Map[A, C] = new DefaultMap[A, C] {
+    override def foreach[D](g: ((A, C)) => D): Unit = for ((k, v) <- self) g((k, f(v)))
+    def iterator = for ((k, v) <- self.iterator) yield (k, f(v))
+    override def size = self.size
+    override def contains(key: A) = self.contains(key)
+    def get(key: A) = self.get(key).map(f)
   }
 
   @deprecated("use `updated' instead")
diff --git a/src/library/scala/collection/immutable/MapProxy.scala b/src/library/scala/collection/immutable/MapProxy.scala
index 0ef7aa620a..84775c9a82 100644
--- a/src/library/scala/collection/immutable/MapProxy.scala
+++ b/src/library/scala/collection/immutable/MapProxy.scala
@@ -38,4 +38,7 @@ trait MapProxy[A, +B] extends Map[A, B] with MapProxyLike[A, B, Map[A, B]]
   override def + [B1 >: B](elem1: (A, B1), elem2: (A, B1), elems: (A, B1) *) =
     newProxy(self.+(elem1, elem2, elems: _*))
   override def -(key: A) = newProxy(self - key)
+
+  override def filterKeys(p: A => Boolean) = self.filterKeys(p)
+  override def mapValues[C](f: B => C) = self.mapValues(f)
 }