Converting deprecated .toIterable

I have some old code which used a .toIterable method which has apparently been deprecated. I’m trying to convert the code and it seems like a somewhat invasive change. Maybe I’m missing something which should make this conversion easy.

  def getClauses(vec: QmVec, posCount: Int, length: Int): Set[List[Int]] = {
    type RECTHASH = mutable.HashMap[ClauseAsList, Set[ClauseAsBitSet]]
    type LENGTHHASH = mutable.HashMap[Int, RECTHASH]
    (for {
      lengthHash: LENGTHHASH <- vec.hash.get(posCount).toIterable
      rectHash: RECTHASH <- lengthHash.get(length).toIterable
      (rectified, clauses) <- rectHash
      clause <- clauses
    } yield bitSetToClause(rectified,clause)).toSet
  }

When I try to simply remove the .toIterable usages, the type of my for comprehension becomes horribly wrong.

Here is what I’ve changed it to: I’ve added type aliases for debugging, but if I take away the local type declarations, I get the same compiler error.

  def getClauses(vec: QmVec, posCount: Int, length: Int): Set[List[Int]] = {
    type RECTHASH = mutable.HashMap[ClauseAsList, Set[ClauseAsBitSet]]
    type LENGTHHASH = mutable.HashMap[Int, RECTHASH]
    (for {
      lengthHash: LENGTHHASH <- vec.hash.get(posCount) // get() returns Option[...]
      rectHash: RECTHASH <- lengthHash.get(length)   // get() returns Option[...]
      (rectified, clauses) <- rectHash
      clause <- clauses
    } yield bitSetToClause(rectified,clause)).toSet
  }
type mismatch;
 found   : scala.collection.mutable.Iterable[dimacs.QmVec.ClauseAsList]
    (which expands to)  scala.collection.mutable.Iterable[List[Int]]
 required: Option[?]
      (rectified, clauses) <- rectHash

The problem seems to be that since the top <- in the for comprehension is an Option[] destructuring, the loop no longer returns a the correct thing.

Do I really need to factor the two Options out of the loop, and chain them with getOrElse? That seems pretty disruptive?

Is there a more obvious way? Should I just convert None to List() and Some(x) to List(x) ?

The following fixes the problem, but seems like I’m doing something wrong.

  def optionToList[T](op:Option[T]):List[T] = {
    op match {
      case None => List[T]()
      case Some(x) => List(x)
    }
  }

  def getClauses(vec: QmVec, posCount: Int, length: Int): Set[List[Int]] = {
    type RECTHASH = mutable.HashMap[ClauseAsList, Set[ClauseAsBitSet]]
    type LENGTHHASH = mutable.HashMap[Int, RECTHASH]
    (for {
      lengthHash: LENGTHHASH <- optionToList(vec.hash.get(posCount))// get() returns Option[...]
      rectHash: RECTHASH <- optionToList(lengthHash.get(length))   // get() returns Option[...]
      (rectified, clauses) <- rectHash
      clause <- clauses
    } yield bitSetToClause(rectified,clause)).toSet
  }

turns out Option also has a toList method which I suppose does the same thing. Perhaps changing toIterator to toList is the easiest fix?

Took a look at the implementation of toList and it is a bit strange. I wonder what the motivation of implementing it in such a bizarre way is.

  def toList: List[A] =
    if (isEmpty) List() else new ::(this.get, Nil)

Probably no motivation except bizarre. The PR was to eliminate :: allocations.

I’m guessing, but it looks to me like an attempt to avoid going through List.apply which is varargs, which costs extra.

1 Like