patternModerate
Code with many "early returns (exits)" into the functional style
Viewed 0 times
exitsthewithintofunctionalstyleearlyreturnsmanycode
Problem
I have some Scala code that uses Internet to authorize a user. Therefore, it can throw Exceptions like
The original code was written in Java. So, yes, it uses variables, not
The following is an example. (The actual code is, of course, a little more complicated.)
I'm trying to rewrite the code in the functional style. What I did was
I tried to use methods like
```
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
}
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
- Use
vals instead ofvars
- Remove "return early"
- Use
Noneinstead ofnull
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
}
Solution
After reading the code, I assume these are the signatures of the building blocks of your code
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 :
The try expression has type Option[(IdInfo, PasswordInfo, ConnectionInfo)] which is the type you want as the output of the function
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
Therefore the return type of the map expression is
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
The original expression can thus be rewritten as:
which has type Option[(IdInfo, PasswordInfo, ConnectionInfo)]. The next step is to study the option type and see that it has a
Then notice that each intermediary is step is of the form :
for instance:
can be rewritten as :
Which means the whole function can now be simplified to :
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 :
then the connect method looks like :
Which is better but not perfect. Let's apply some scala syn
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 NoneThe 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
}
}) flattenwhich 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] } flattenfor instance:
connInfoOpt map (connInfo => {
try {
verify(idInfo, pwdInfo, connInfo)
Some(idInfo, pwdInfo, connInfo)
} catch {
case ex: IOException => None
case ex: AuthorizationException => None
}
}) flattencan 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 syn
Code Snippets
class IdInfo
class PasswordInfo
class ConnectionInfo
def calcIdInfo:IdInfo=???
def calcPwdInfo(idInfo:IdInfo):PasswordInfo=???
def calcConnInfo(idInfo:IdInfo,pwdInfo:PasswordInfo):PasswordInfo=???connInfoOpt map (connInfo => {
try {
verify(idInfo, pwdInfo, connInfo)
Some(idInfo, pwdInfo, connInfo)
} catch {
case ex: IOException => None
case ex: AuthorizationException => None
}
}) getOrElse Nonemap(f:A=>B):Option[B]A = ConnectionInfo
B = Option[(IdInfo, PasswordInfo, ConnectionInfo)]Option[Option[(IdInfo, PasswordInfo, ConnectionInfo)]]Context
StackExchange Code Review Q#38689, answer score: 19
Revisions (0)
No revisions yet.