Code with many "early returns (exits)" into the functional style

Question

I have some Scala code that uses Internet to authorize a user. Therefore, it can throw Exceptions like IOException in the method.

The original code was written in Java. So, yes, it uses variables, not val. Also it "returns early" many times if some condition is not satisfied.

The following is an example. (The actual code is, of course, a little more complicated.)

def connect(): (IdInfo, PasswordInfo, ConnectionInfo) = {
  var idInfo: IdInfo = null
  var pwdInfo: PasswordInfo = null
  var connInfo: ConnectionInfo = null

  try {
    idInfo = calcIdInfo()
  } catch {
    case ex: IOException => return null
  }

  try {
    pwdInfo = calcPwdInfo(idInfo)
  } catch {
    case ex: AuthorizationException => return null
  }

  try {
    connInfo = calcConnInfo(idInfo, pwdInfo)
  } catch {
    case ex: IOException => return null
  }

  try {
    verify(idInfo, pwdInfo, connInfo)
  } catch {
    case ex: IOException => return null
    case ex: AuthorizationException => return null
  }

  (idInfo, pwdInfo, connInfo)
}

I'm trying to rewrite the code in the functional style. What I did was

1. Use vals instead of vars
2. Remove "return early"
3. Use None instead of null

I tried to use methods like fold, but at each stage, the types of input or the types of output are different - IdInfo, (IdInfo, PasswordInfo), (IdInfo, PasswordInfo, ConnectionInfo) are all different types - and, what each stage does is also diffrent from each other, so I couldn't use such method.

def connect(): Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {
  val idInfoOpt = try {
    Some(calcIdInfo())
  } catch {
    case ex: IOException => None
  }

  idInfoOpt map (idInfo => {
    val pwdInfoOpt = try {
      Some(calcPwdInfo(idInfo))
    } catch {
      case ex: AuthorizationException => None
    }

    pwdInfoOpt map (pwdInfo => {
      val connInfoOpt = try {
        Some(calcConnInfo(idInfo, pwdInfo))
      } catch {
        case ex: IOException => None
      }

      connInfoOpt map (connInfo => {
        try {
          verify(idInfo, pwdInfo, connInfo)
          Some(idInfo, pwdInfo, connInfo)
        } catch {
          case ex: IOException => None
          case ex: AuthorizationException => None
        }
      }) getOrElse None
    }) getOrElse None
  }) getOrElse None
}

The resultant code is more complex than the original one. There are deeper indentations, and the type of each value is not easy to determine (A lot of Options and map and getOrElse). In the above example, the code has calc***Info() methods, but in reality the code is more complex since it has more statements between each stage, so there seems almost no reason to use the actual resultant code instead of the original one.

I think that the procedural style is better than the functional style in this case. Or not? The more complex the code is, the more chance of errors there will be. Are there better ways to get rid of vars and early returns?

For ease, here are compilable source codes (although it does nothing):
The first one: http://ideone.com/KJS1tn
The second one: http://ideone.com/gulzgs

Answer 1

After reading the code, I assume these are the signatures of the building blocks of your code

class IdInfo
class PasswordInfo
class ConnectionInfo

def calcIdInfo:IdInfo=???
def calcPwdInfo(idInfo:IdInfo):PasswordInfo=???
def calcConnInfo(idInfo:IdInfo,pwdInfo:PasswordInfo):PasswordInfo=???

I left out the implementations for the calc as they don't really matter.

There are several possible variations to improve on the current code. Let's have a look at the last call to map in connect :

 connInfoOpt map (connInfo => {
    try {
      verify(idInfo, pwdInfo, connInfo)
      Some(idInfo, pwdInfo, connInfo)
    } catch {
      case ex: IOException => None
      case ex: AuthorizationException => None
    }
  }) getOrElse None

The try expression has type Option[(IdInfo, PasswordInfo, ConnectionInfo)] which is the type you want as the output of the function connect. As the name connInfoOpt suggests, it is also an option. Map on option has a signature roughly equivalent to :

map(f:A=>B):Option[B]

Where A is the type which is wrapped in the original option, and B is the type wrapped in the resulting option.

Thus let's say connInfoOpt has type Option[ConnectionInfo] we get

A = ConnectionInfo
B = Option[(IdInfo, PasswordInfo, ConnectionInfo)]

Therefore the return type of the map expression is Option[B] which can be expanded to

Option[Option[(IdInfo, PasswordInfo, ConnectionInfo)]]

Here lies the first problem : to get the Option[(IdInfo, PasswordInfo, ConnectionInfo)] out, you use getOrElse None. A slightly shorter way to express that is to call flatten.

The original expression can thus be rewritten as:

 connInfoOpt map (connInfo => {
    try {
      verify(idInfo, pwdInfo, connInfo)
      Some(idInfo, pwdInfo, connInfo)
    } catch {
      case ex: IOException => None
      case ex: AuthorizationException => None
    }
  }) flatten

which has type Option[(IdInfo, PasswordInfo, ConnectionInfo)]. The next step is to study the option type and see that it has a flatMap method which has the following signature :

flatMap(f:A=>Option[B]):Option[B]
// for comparison with map which was 
map(f:A=>B):Option[B]

Then notice that each intermediary is step is of the form :

option map { A=> Option[B] } flatten

for instance:

connInfoOpt map (connInfo => {
  try {
    verify(idInfo, pwdInfo, connInfo)
    Some(idInfo, pwdInfo, connInfo)
  } catch {
    case ex: IOException => None
    case ex: AuthorizationException => None
  }
}) flatten

can be rewritten as :

connInfoOpt flatMap (connInfo => {
  try {
    verify(idInfo, pwdInfo, connInfo)
    Some(idInfo, pwdInfo, connInfo)
  } catch {
    case ex: IOException => None
    case ex: AuthorizationException => None
  }
})

Which means the whole function can now be simplified to :

 def connect(): Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {
    val idInfoOpt = try {
      Some(calcIdInfo())
    } catch {
      case ex: IOException => None
    }

    idInfoOpt flatMap (idInfo => {
      val pwdInfoOpt = try {
        Some(calcPwdInfo(idInfo))
      } catch {
        case ex: AuthorizationException => None
      }

      pwdInfoOpt flatMap (pwdInfo => {
        val connInfoOpt = try {
          Some(calcConnInfo(idInfo, pwdInfo))
        } catch {
          case ex: IOException => None
        }

        connInfoOpt flatMap (connInfo => {
          try {
            verify(idInfo, pwdInfo, connInfo)
            Some(idInfo, pwdInfo, connInfo)
          } catch {
            case ex: IOException => None
            case ex: AuthorizationException => None
          }
        })
      })
    })
  }

We got rid of the 3 getOrElse but this still isn't so nice. Let's extract a few methods next to make the code easier to read :

def safeIdInfo():Option[IdInfo]={
  try {
    Some(calcIdInfo())
  } catch {
    case ex: IOException => None
  }
}

def verifyConnInfo(idInfo: IdInfo, pwdInfo: PasswordInfo, connInfo: ConnectionInfo) :Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {
  try {
    verify(idInfo, pwdInfo, connInfo)
    Some(idInfo, pwdInfo, connInfo)
  } catch {
    case ex: IOException => None
    case ex: AuthorizationException => None
  }
}

def safeConnInfo(idInfo: IdInfo, pwdInfo: PasswordInfo): Option[ConnectionInfo] = {
  try {
    Some(calcConnInfo(idInfo, pwdInfo))
  } catch {
    case ex: IOException => None
  }
}

def safePwdInfo(idInfo: IdInfo): Option[PasswordInfo] = {
  try {
    Some(calcPwdInfo(idInfo))
  } catch {
    case ex: AuthorizationException => None
  }
}

then the connect method looks like :

def connect(): Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {
  safeIdInfo() flatMap (idInfo =>
    safePwdInfo(idInfo) flatMap (pwdInfo=>
      safeConnInfo(idInfo, pwdInfo) flatMap (connInfo =>
        verifyConnInfo(idInfo, pwdInfo, connInfo)
      )
    )
  )
}

Which is better but not perfect. Let's apply some scala syntactic sugar :

def connect(): Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {    
  for{
    idInfo   <- safeIdInfo()
    pwdInfo  <- safePwdInfo(idInfo)
    connInfo <- safeConnInfo(idInfo, pwdInfo)
    verified <- verifyConnInfo(idInfo, pwdInfo, connInfo)
  } yield verified 
}  

The connect method is much better (I think) but we end up with repetition in the "safe" methods. Actually, Scala offers a type to reduce this duplication, the Try type which we will use to replace the Option type in the "safe" methods :

def verifyConnInfo(idInfo: IdInfo, pwdInfo: PasswordInfo, connInfo: ConnectionInfo) :Try[(IdInfo, PasswordInfo, ConnectionInfo)] = Try {
  verify(idInfo, pwdInfo, connInfo)
  (idInfo, pwdInfo, connInfo)
}
def safeIdInfo():Try[IdInfo]=Try(calcIdInfo())    
def safeConnInfo(idInfo: IdInfo, pwdInfo: PasswordInfo): Try[ConnectionInfo] = Try(calcConnInfo(idInfo, pwdInfo))
def safePwdInfo(idInfo: IdInfo): Try[PasswordInfo] = Try(calcPwdInfo(idInfo))

Of course doing this, the connect method is no longer valid at the type level it needs to be adapted. To match the exact behavior of the original code the adaptation will look like :

  def connect(): Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {    
    val tryVerified = for{
      idInfo <- safeIdInfo()
      pwdInfo <-  safePwdInfo(idInfo)
      connInfo <- safeConnInfo(idInfo, pwdInfo)
      verified <- verifyConnInfo(idInfo, pwdInfo, connInfo)
    } yield verified

    tryVerified match {
      case Success(value) => Some(value)
      case Failure(authException:AuthorizationException)=> None
      case Failure(ioException:IOException)=> None
      case Failure(failed)=> throw failed
    }
  }  

However I will suppose that you don't really want to rethrow unknown runtime exceptions and instead want to return None for any error, in which case the following form of connect should work :

  def connect(): Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {    
    val tryVerified = for{
      idInfo <- safeIdInfo()
      pwdInfo <-  safePwdInfo(idInfo)
      connInfo <- safeConnInfo(idInfo, pwdInfo)
      verified <- verifyConnInfo(idInfo, pwdInfo, connInfo)
    } yield verified

    tryVerified.toOption
  }  

The complete code is now :

 def verifyConnInfo(idInfo: IdInfo, pwdInfo: PasswordInfo, connInfo: ConnectionInfo) :Try[(IdInfo, PasswordInfo, ConnectionInfo)] = Try {
    verify(idInfo, pwdInfo, connInfo)
    (idInfo, pwdInfo, connInfo)
  }
  def safeIdInfo():Try[IdInfo]=Try(calcIdInfo())
  def safeConnInfo(idInfo: IdInfo, pwdInfo: PasswordInfo): Try[ConnectionInfo] = Try(calcConnInfo(idInfo, pwdInfo))
  def safePwdInfo(idInfo: IdInfo): Try[PasswordInfo] = Try(calcPwdInfo(idInfo))

  def connect(): Option[(IdInfo, PasswordInfo, ConnectionInfo)] = {    
    val tryVerified = for{
      idInfo <- safeIdInfo()
      pwdInfo <-  safePwdInfo(idInfo)
      connInfo <- safeConnInfo(idInfo, pwdInfo)
      verified <- verifyConnInfo(idInfo, pwdInfo, connInfo)
    } yield verified

    tryVerified.toOption
  }  

Using the right types, a functional style and some syntactic sugar can go a long way to simplify your code :)

For further reference, check the scala doc for Option and Try, also check out the FAQ on yield for more explanations on the for-expression syntactic sugar.