Call a method on the value of Scala Option if present

Question

I am trying to figure out the best way to refactor the following code to eliminate the use of Option.get(). I know that using the get method is considered bad practice.

if (myConnection.isDefined) {
  myConnection.get.close
}

where myConnection is of type Option[Connection]

getOrElse doesn't seem like it would work, because there is no "else" object to call the method on. If myConnection is None, then I don't want to do anything.

I suppose I could use forEach like:

myConnection.foreach{ c => c.close }

This would work, but it looks weird to me. In my case, myConnection would never contain more than one connection, and someone else later looking at my code might be led to believe that it could contain multiple connections.

Is there a better way to do this that is both concise and clear?

Using `foreach` is already concise and clear to an experienced Scala programmer, so don't worry about other programmers getting confused. I would write `myConnection.foreach(_.close)`, but otherwise this is the natural way to write this code. — Tim, Nov 21 '18 at 20:04
I agree, there is nothing to worry about. Foreach is just fine and you will get used to it too. Soon, you will find yourself writing future.foreach, knowing exactly what it means. — ygor, Nov 21 '18 at 20:59
I agree. It looks weird and is bad for readability because it gives the impression that you are traversing a collection when you don't. It's intentional to be consistent with the collection API, but that's also what makes it misleading. The only place where Java has done a better job with its Optional counterpart, by calling the method 'ifPresent' — stackunderflow, Feb 14 '23 at 16:21

prayagupa · Answer 1 · 2018-11-21T19:25:05.283

foreach makes sense when you are doing computation with return valueunit, side-effects. Closing a connection sounds good case for foreach to me.

This is what Option.foreach looks like:

@inline final def foreach[U](f: A => U) {
  if (!isEmpty) f(this.get)
}

But if you want to do some computation and return the value, .map or match could be better.

import scala.util.Try

val connectionMaybe = Try {
  DriverManager.getConnection(
    s"jdbc:h2:~/test;MODE=Oracle",
    "sa",
    ""
  )
}.toOption


def getSomething(connectionMaybe: Option[Connection]): Option[Int] = {
  connectionMaybe match {
    case Some(connection) =>
      val statement = connection.createStatement()
      val rs = statement.executeQuery(s"select * from something")
      Option(rs.getInt("some_column"))
      //cleanup if needed
    case _ =>
      println("no connection found")
      None
  }
}

score 2 · Accepted Answer · answered Nov 21 '18 at 19:15

Typically, you would map over an Option to operate on its value.

myConnection.map(c => c.close())

or

myConnection.map(close(_))

which will do nothing if c is None and will return a None. Otherwise, it give you a Some(True) if, for example, close() returns a True when it succeeds. If close() had no return value, then mapping will return a Some(()) which has the type Option[Unit].

score 1 · Answer 3 · answered Nov 21 '18 at 20:47

1

Pattern-matching version if you don't like pure functional foreach/map calls:

myConnection match {
  case Some(conn) => 
    conn // TODO add logic here, `con` is unwrapped here
  case None => 
    // TODO add error-back logic here
}

answered Nov 21 '18 at 20:47

morsik

1,250
14
17

Call a method on the value of Scala Option if present

3 Answers3