Yielding Unit in for comprehension

I expected an error from the compiler on the following, but it was accepted happily. Is this code creating a sequence of Units and then throwing it away? This code was actually an intermediate step in a code refactoring. But since the compiler does not complain, I’m tempted to leave it as-is.

    def mapClauses(consume: Clause => Unit): Unit = {
      for {
        (_, lengthHash) <- hash
        (_, clauses) <- lengthHash
      } yield clauses.foreach(consume)
    }

hash is a Map here? Yeah, it looks like the for comprehension is returning an Iterable[Unit], which is legitimate but useless. mapClauses() would return that, but since you’ve explicitly said that it returns Unit that’s simply being thrown away.

Basically, ascribing Unit as the return type means exactly the same thing as the old procedure syntax (and is some folks’ preferred way of expressing this idea) – it says that this function doesn’t return anything, and whatever value you have at the bottom should be discarded. I believe it’s always legal, regardless of what the actual value produced inside the function is.

It would be great if there were a warning that I’m producing a result and then discarding it. Is this out of the scope of the compiler? Perhaps it’s a job for IntelliJ?

yes, hash is a Map, actually a HashMap. Changing to the following:

import scala.collection.mutable.HashMap

object sample {
    type Clause = List[Int]
    type Clauses = List[Clause]
    val hash: HashMap[Int, HashMap[Int, Clauses]] = new HashMap

    def mapClauses(consume: Clause => Unit): Unit = {
      hash.foreach{case (_,lengthHash)=>
        lengthHash.foreach{case (_,clauses)=>
          clauses.foreach{consume}
        }
      }
    }
}

There’s a compiler flag for that: -Ywarn-value-discard

4 Likes

As @Jasper-M says, there’s a flag to do that. But keep in mind, you’re explicitly saying you want to discard the results – that’s basically what : Unit means. So it shouldn’t be too surprising that the compiler goes along with that unless you say otherwise.

(This is, BTW, the primary argument for getting rid of procedure syntax – which is what happens when you simply leave off the = in a def – which may happen in Scala 3. It has the same connotation of “discard the result”, but is more visually subtle and easier to do by accident, so some of us dislike it.)