Refactor toMultimap

[zsxwing]

This PR includes the following breaking changes:

• Change the return type from Observable[scala.collection.Map[K, Seq[T]]] to Observable[mutable.MultiMap[K, V]].
• Change the method name toMultimap to toMultiMap to make it consistent to the return type.
• Remove toMultimap(keySelector, valueSelector, mapFactory, bufferFactory). You can override MultiMap.makeSet to create your custom bufferFactory. E.g.,

     val o : Observable[String] = List("alice", "bob", "carol", "allen", "clarke").toObservable
     val keySelector = (s: String) => s.head
     val valueSelector = (s: String) => s.tail
     val mapFactory = () => new mutable.HashMap[Char, mutable.Set[String]] with mutable.MultiMap[Char, String] {
       override def makeSet = new mutable.TreeSet[String]
     }
     val m = o.toMultiMap(keySelector, valueSelector, mapFactory)
     println(m.toBlocking.single)

[samuelgruetter]

• I think a by name parameter (i.e. multiMapFactory: => M) would be more scala-ish than multiMapFactory: () => M.
• Can you add default arguments for multiMapFactory and maybe even for valueSelector? Then you could remove some overloads and get a smaller API.

[zsxwing]

Default value for multiMapFactory does not work. If I change it to

  def toMultiMap[K, V, M <: mutable.MultiMap[K, V]](
       keySelector: T => K,
       valueSelector: T => V,
       multiMapFactory: => M = new mutable.HashMap[K, mutable.Set[V]] with mutable.MultiMap[K, V]): Observable[M] = {

There will be some error such as

RxScala/src/test/scala/rx/lang/scala/ObservableTest.scala:242: type mismatch;
[error]  found   : String => String
[error]  required: String => V
[error]     val o = Observable.just("a", "b", "cc", "dd").toMultiMap(_.length, s => s + s)

Looks the compiler can not infer the type V from _.length, s => s + s and the default value new mutable.HashMap[String, mutable.Set[V]] with mutable.MultiMap[String, V].

[zsxwing]

When I saw the following signature,

toMultiMap[K, V, M <: mutable.MultiMap[K, V]](keySelector: T => K, valueSelector: T => V, multiMapFactory: => M)

I thought it might be better to use currying for multiMapFactory.
So I tried the following two ways:
1.

toMultiMap[K, V, M <: mutable.MultiMap[K, V]](multiMapFactory: => M)(keySelector: T => K, valueSelector: T => V)

It does not work because

RxScala/src/main/scala/rx/lang/scala/Observable.scala:3990: overloaded method value toMultiMap with alternatives:
[error]   [K, V, M <: scala.collection.mutable.MultiMap[K,V]](multiMapFactory: => M)(keySelector: T => K, valueSelector: T => V)rx.lang.scala.Observable[M] <and>
[error]   [K, V >: T](keySelector: T => K)rx.lang.scala.Observable[scala.collection.mutable.MultiMap[K,V]]
[error]  cannot be applied to (scala.collection.mutable.HashMap[K,scala.collection.mutable.Set[V]] with scala.collection.mutable.MultiMap[K,V])
[error]     toMultiMap(new mutable.HashMap[K, mutable.Set[V]] with mutable.MultiMap[K, V])(keySelector, valueSelector)
[error]     ^

toMultiMap[K, V, M <: mutable.MultiMap[K, V]](keySelector: T => K, valueSelector: T => V)(multiMapFactory: => M)

It also does not work because

RxScala/src/main/scala/rx/lang/scala/Observable.scala:3990: ambiguous reference to overloaded definition,
[error] both method toMultiMap in trait Observable of type [K, V, M <: scala.collection.mutable.MultiMap[K,V]](keySelector: T => K, valueSelector: T => V)(multiMapFactory: => M)rx.lang.scala.Observable[M]
[error] and  method toMultiMap in trait Observable of type [K, V](keySelector: T => K, valueSelector: T => V)rx.lang.scala.Observable[scala.collection.mutable.MultiMap[K,V]]
[error] match argument types (T => K,T => V)
[error]     toMultiMap(keySelector, valueSelector)(new mutable.HashMap[K, mutable.Set[V]] with mutable.MultiMap[K, V])
[error]     ^

At last, looks the following signature is best now:

toMultiMap[K, V, M <: mutable.MultiMap[K, V]](keySelector: T => K, valueSelector: T => V, multiMapFactory: => M)

[zsxwing]

@samuelgruetter further suggestion?

[samuelgruetter]

LGTM.
Too bad we can't have the curried version, but I didn't find anything either.
And I think it's acceptable that in toMap, we use CanBuildFrom, whereas in toMultiMap, we use a mapFactory, because Scala's MultiMap was not designed to work with CanBuildFrom, so it's not "our fault".

[samuelgruetter]

@zsxwing could you please write some release notes giving a summary about the changes of toMap and toMultiMap? So that users know how to update their code, and why certain methods were removed.

[zsxwing]

@zsxwing could you please write some release notes giving a summary about the changes of toMap and toMultiMap? So that users know how to update their code, and why certain methods were removed.

OK.