JoeMjr2
JoeMjr2

Reputation: 3944

Call a method on the value of Scala Option if present

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?

Upvotes: 8

Views: 8141

Answers (3)

morsik
morsik

Reputation: 1300

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
}

Upvotes: 1

prayagupadhyay
prayagupadhyay

Reputation: 31192

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
  }
}

Upvotes: 5

Metropolis
Metropolis

Reputation: 2128

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].

Upvotes: 3

Related Questions