2
\$\begingroup\$

I'd like to simplify this Scala code:

private def getToken() = {
    val token = getTokenFromDb()
    token match {
      case Some(x) => token
      case None => {
        val remoteToken = getTokenRemote()
        remoteToken match {
          case Some(x) => {
            writeTokenToDb(x)
            remoteToken
          }
          case None => None
        }
      }
    }
  }
\$\endgroup\$
4
\$\begingroup\$
private def getToken(): Option[Token] = {
  def remoteToken =
    getTokenRemote() map { r => writeTokenToDb(r); r }
  getTokenFromDb orElse remoteToken
}

Please write always the type of a function for outer functions - it makes it far more readable to understand the source code (often it is useful to add the types to inner functions/declarations as well).

Beside from that you can also write case t: Some => t or case t @ Some(_) => t to express that you return the matched type. Your way of case Some(x) => token is hard to understand, because one needs to know what's token.

Furthermore, you don't need braces around a match block, you can simply write

case x =>
  a
  b

And naming a variable in a similar way as a function, like in val token = getToken() is completely unnecessary, when it is only used once, it also doesn't increase readability. In such cases just inline the function call: getToken match { ... }.

\$\endgroup\$
  • \$\begingroup\$ it seems like you make an unnecessary recursive call in def getToken(). How can it return Some(x) in ` getToken orElse remoteToken ? It will call itself recursively. \$\endgroup\$ – アレックス Nov 3 '13 at 5:48
  • \$\begingroup\$ it's my miss, it should be getTokenFromDb, shouldn't it? \$\endgroup\$ – アレックス Nov 3 '13 at 5:52
  • \$\begingroup\$ @Alex: You are right, that was a mistake. \$\endgroup\$ – kiritsuku Nov 3 '13 at 10:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.