sbt/sbt-remote-control

protocol/Keys.scala uses scala.Seq, not immutable.Seq.

Opened this issue · 3 comments

In the wip/jsuereth-model-view-server branch, scala.Seq is used, like in Keys.scala. It would be better to use scala.collection.immutable.Seq.

Why is it better? Just curious....

Really?

scala.Seq is a type alias to scala.collection.Seq, so it cannot be assumed to be immutable. It leads to some fun code:

  case class Foo(s: Seq[Int]) {
    def sum = s.sum
  }

  val s = ListBuffer(1, 2)                        //> s  : scala.collection.mutable.ListBuffer[Int] = ListBuffer(1, 2)
  val f = Foo(s)                                  //> f  : testa.Foo = Foo(ListBuffer(1, 2))

  f.sum                                           //> res0: Int = 3
  s.append(3)
  f.sum                                           //> res1: Int = 6

  val f2 = Foo(s.to[List])                        //> f2  : testa.Foo = Foo(List(1, 2, 3))

  f == f2                                         //> res2: Boolean = true
  s.append(4)
  f == f2                                         //> res3: Boolean = false

True, but it's ok to mutate something locally and expose an immutable interface to it as long as you give up control of the mutability once you pass the reference to someone. It's one of those nice optimisation tricks. Again, mutability should never escape a single method/object and then you're ok.

Practically, anyone could implement immutable.Seq with a mutable collection, it's just evil to do so. As is passing a mutable collection to something taking Seq[_] and then mutating later :)

In this case though, I may be being pedantic, because I don't expect users to construct any of the values in Keys. Keys represents a read-only view of data for clients. Personally I still prefer collection.Seq for flexibility in local implementations.