Finding myself in an imperative situation; would like to see an alternative approach to the following (which does not work due to the use of return keyword and lack of explicit type on method):
def save = Action { implicit request =>
val bound = form.bindFromRequest
if(bound.hasErrors) return Ok(withForm(bound))
val model = bound.get
val orderNum = generateOrderNum()
val transID = processPayment(model, orderNum)
if(transID.isEmpty) return Ok(withForm( bound.withGlobalError(...) ))
val ip = request.headers.get("X-Forwarded-For")
val result = dao.createMember(model, ip, orderNum, transID)
result match {
case Left(_) =>
Ok(withForm( bound.withGlobalError(...) ))
case Right((foo, bar, baz)) =>
// all good: generate pdf, email, redirect with success msg
}
}
}
In this case I like the use of return as you avoid nesting several if/else blocks, or folds, or matches, or fill-in-the-blank non-imperative approach. The problem of course, is that it doesn't work, an explicit return type has to specified, which has its own issues as I have yet to figure out how to specify a type that satisfies whatever Play magic is at work -- no, def save: Result
, does not work as the compiler then complains about implicit result
now not having an explicit type ;-(
At any rate, Play framework examples provide la, la, la, la happy 1-shot-deal fold(error, success) condition which is not always the case in the real world™ ;-)
So what is the idiomatic equivalent (without use of return) to above code block? I assume it would be nested if/else, match, or fold, which gets a bit ugly, indenting with each nested condition.
References:
Scala return keyword
handling errors in scala controllers
EDIT3
This is the "final" solution, again thanks to Dan Burton.
def save = Action { implicit request =>
val(orderNum, ip) = (generateOrderNum, request.remoteAddress)
val result = for {
model <- bindForm(form).right // error condition already json'd
transID <- payment.process(model, orderNum) project json
userID <- dao.create(model, ip, orderNum, transID) project json
} yield (userID, transID)
}
Then the pimp'd Either project method, placed somewhere in your application (in my case, an implicits trait that sbt root & child project(s) extends their base package object from:
class EitherProvidesProjection[L1, R](e: Either[L1, R]) {
def project[L1, L2](f: L1 => L2) = e match {
case Left(l:L1) => Left(f(l)).right
case Right(r) => Right(r).right
}
}
@inline implicit final def either2Projection[L,R](e: Either[L,R]) = new EitherProvidesProjection(e)
EDIT2
Evolution, have gone from embedded return statements to this little white dwarf of density (kudos to @DanBurton, the Haskell rascal ;-))
def save = Action { implicit request =>
val(orderNum, ip) = (generateOrderNum, request.remoteAddress)
val result = for {
model <- form.bindFromRequest fold(Left(_), Right(_)) project( (f:Form) => Conflict(f.errorsAsJson) )
transID <- payment.process(model, orderNum) project(Conflict(_:String))
userID <- dao.create(model, ip, orderNum, transID) project(Conflict(_:String))
} yield (userID, transID)
...
}
I have added Dan's onLeft Either projection as a pimp to Either, with the above "project" method, which allows for right-biased eitherResult project(left-outcome)
. Basically you get fail-first error as a Left and success as a Right, something that would not work when feeding Option outcomes to for comprehension (you only get Some/None outcome).
The only thing I'm not thrilled with is having to specify the type for the project(Conflict(param))
; I thought the compiler would be able to infer the left condition type from the Either that is being passed to it: apparently not.
At any rate, it's clear that the functional approach obviates the need for embedded return statements as I was trying to do with if/else imperative approach.
EDIT
The functional equivalent is:
val bound = form.bindFromRequest
bound fold(
error=> withForm(error),
model=> {
val orderNum = generateOrderNum()
payment.process(model, orderNum) fold (
whyfail=> withForm( bound.withGlobalError(whyfail) ),
transID=> {
val ip = request.headers.get("X-Forwarded-For")
dao.createMember(model, ip, orderNum, transID) fold (
errcode=>
Ok(withForm( bound.withGlobalError(i18n(errcode)) )),
userID=>
// generate pdf, email, redirect with flash success
)}
)}
)
which is certainly a densely power packed block of code, a lot happening there; however, I would argue that corresponding imperative code with embedded returns is not only similarly concise, but also easier to grok (with added benefit of fewer trailing curlies & parens to keep track of)
So as a Haskeller, obviously in my mind, the solution to everything is Monads. Step with me for a moment into a simplified world (simplified for me, that is) where your problem is in Haskell, and you have the following types to deal with (as a Haskeller, I sort of have this fetish for types):
bindFormRequest :: Request -> Form -> BoundForm
hasErrors :: BoundForm -> Bool
processPayment :: Model -> OrderNum -> TransID
isEmpty :: TransID -> Bool
Let's pause here. At this point, I'm sort of cringing a bit at boundFormHasErrors
and transIDisEmpty
. Both of these things imply that the possibility of failure is injected into BoundForm
and TransID
respectively. That's bad. Instead, the possibility of failure should be maintained separate. Allow me to propose this alternative:
bindFormRequest :: Request -> Form -> Either FormBindError BoundForm
processPayment :: Model -> OrderNum -> Either TransError TransID
That feels a bit better, and these Eithers are leading into the use of the Either monad. Let's write up some more types though. I'm going to ignore OK
because that is wrapped around pretty much everything; I'm fudging a little bit but the concepts will still translate just the same. Trust me; I'm bringing this back around to Scala in the end.
save :: Request -> IO Action
form :: Form
withForm :: BoundForm -> Action
getModel :: BoundForm -> Model
generateOrderNum :: IO OrderNum
withGlobalError :: ... -> BoundForm -> BoundForm
getHeader :: String -> Request -> String
dao :: DAO
createMember :: Model -> String -> OrderNum -> TransID
-> DAO -> IO (Either DAOErr (Foo, Bar, Baz))
allGood :: Foo -> Bar -> Baz -> IO Action
OK, now I'm going to do something a bit wonky, and let me tell you why. The Either monad works like this: as soon as you hit a Left
, you stop. (Is it any surprise I chose this monad to emulate early returns?) This is all well and good, but we want to always stop with an Action
, and so stopping with a FormBindError
isn't going to cut it. So let's define two functions that will let us deal with Eithers in such a way that we can install a little more "handling" if we discover a Left
.
-- if we have an `Either a a', then we can always get an `a' out of it!
unEither :: Either a a -> a
unEither (Left a) = a
unEither (Right a) = a
onLeft :: Either l r -> (l -> l') -> Either l' r
(Left l) `onLeft` f = Left (f l)
(Right r) `onLeft` _ = Right r
At this point, in Haskell, I would talk about monad transformers, and stacking EitherT
on top of IO
. However, in Scala, this is not a concern, so wherever we see IO Foo
, we can just pretend it is a Foo
.
Alright, let's write save
. We will use do
syntax, and later will translate it to Scala
's for
syntax. Recall in for
syntax you are allowed to do three things:
<-
(this is comparable to Haskell's <-
)=
(this is comparable to Haskell's let
)if
(this is comparable to Haskell's guard
function, but we won't use this because it doesn't give us control of the "exceptional" value produced)And then at the end we can yield
, which is the same as return
in Haskell. We will restrict ourselves to these things to make sure that the translation from Haskell to Scala is smooth.
save :: Request -> Action
save request = unEither $ do
bound <- bindFormRequest request form
`onLeft` (\err -> withForm (getSomeForm err))
let model = getModel bound
let orderNum = generateOrderNum
transID <- processPayment model orderNum
`onLeft` (\err -> withForm (withGlobalError ... bound))
let ip = getHeader "X-Forwarded-For" request
(foo, bar, baz) <- createMember model ip orderNum transID dao
`onLeft` (\err -> withForm (withGlobalError ... bound))
return $ allGood foo bar baz
Notice something? It looks almost identical to the code you wrote in imperative style!
You may be wondering why I went through all this effort to write up an answer in Haskell. Well, it's because I like to typecheck my answers, and I'm rather familiar with how to do this in Haskell. Here's a file that typechecks, and has all of the type signatures I just specified (sans IO
): http://hpaste.org/69442
OK, so now let's translate that to Scala. First, the Either
helpers.
Here begins the Scala
// be careful how you use this.
// Scala's subtyping can really screw with you if you don't know what you're doing
def unEither[A](e: Either[A, A]): A = e match {
case Left(a) => a
case Right(a) => a
}
def onLeft[L1, L2, R](e: Either[L1, R], f: L1 => L2) = e match {
case Left(l) = Left(f(l))
case Right(r) = Right(r)
}
Now, the save
method
def save = Action { implicit request => unEither( for {
bound <- onLeft(form.bindFormRequest,
err => Ok(withForm(err.getSomeForm))).right
model = bound.get
orderNum = generateOrderNum()
transID <- onLeft(processPayment(model, orderNum),
err => Ok(withForm(bound.withGlobalError(...))).right
ip = request.headers.get("X-Forwarded-For")
(foo, bar, baz) <- onLeft(dao.createMember(model, ip, orderNum, transID),
err => Ok(withForm(bound.withGlobalError(...))).right
} yield allGood(foo, bar, baz) ) }
Note that variables on the left hand side of <-
or =
are implicitly considered to be val
s since they are inside of a for
block. You should feel free to change onLeft
so that it is pimped onto Either
values for prettier usage. Also, make sure you import an appropriate "Monad instance" for Either
s.
In conclusion, I just wanted to point out that the whole purpose of monadic sugar is to flatten out nested functional code. So use it!
[edit: in Scala, you have to "right bias" Either
s to make them work with for
syntax. This is done by adding .right
to the Either
values on the right-hand side of the <-
. No extra imports necessary. This could be done inside of onLeft
for prettier-looking code. See also: https://stackoverflow.com/a/10866844/208257 ]