How to accumulate errors in Either?
You want to isolate the test*
methods and stop using a comprehension!
Assuming (for whatever reason) that scalaz isn't an option for you... it can be done without having to add the dependency.
Unlike a lot of scalaz examples, this is one where the library doesn't reduce verbosity much more than "regular" scala can:
def testPerson(person: Person): Either[List[String], Person] = {
val name = testPersonName(person.name)
val addr = testAddress(person.address)
val phone = testPhone(person.phone)
val errors = List(name, addr, phone) collect { case Left(err) => err }
if(errors.isEmpty) Right(person) else Left(errors)
}
As @TravisBrown is telling you, for comprehensions don't really mix with error accumulations. In fact, you generally use them when you don't want fine grained error control.
A for comprehension will "short-circuit" itself on the first error found, and this is almost always what you want.
The bad thing you are doing is using String
to do flow control of exceptions. You should at all times use Either[Exception, Whatever]
and fine tune logging with scala.util.control.NoStackTrace
and scala.util.NonFatal
.
There are much better alternatives, specifically:
scalaz.EitherT
and scalaz.ValidationNel
.
Update:(this is incomplete, I don't know exactly what you want). You have better options than matching, such as getOrElse
and recover
.
def testPerson(person: Person): Person = {
val attempt = Try {
val pn = testPersonName(person.name)
val a = testAddress(person.address)
testPhone(person.phone)
}
attempt match {
case Success(person) => //..
case Failure(exception) => //..
}
}
Scala's for
-comprehensions (which desugar to a combination of calls to flatMap
and map
) are designed to allow you to sequence monadic computations in such a way that you have access to the result of earlier computations in subsequent steps. Consider the following:
def parseInt(s: String) = try Right(s.toInt) catch {
case _: Throwable => Left("Not an integer!")
}
def checkNonzero(i: Int) = if (i == 0) Left("Zero!") else Right(i)
def inverse(s: String): Either[String, Double] = for {
i <- parseInt(s).right
v <- checkNonzero(i).right
} yield 1.0 / v
This won't accumulate errors, and in fact there's no reasonable way that it could. Suppose we call inverse("foo")
. Then parseInt
will obviously fail, which means there's no way we can have a value for i
, which means there's no way we could move on to the checkNonzero(i)
step in the sequence.
In your case your computations don't have this kind of dependency, but the abstraction you're using (monadic sequencing) doesn't know that. What you want is an Either
-like type that isn't monadic, but that is applicative. See my answer here for some details about the difference.
For example, you could write the following with Scalaz's Validation
without changing any of your individual validation methods:
import scalaz._, syntax.apply._, syntax.std.either._
def testPerson(person: Person): Either[List[String], Person] = (
testPersonName(person.name).validation.toValidationNel |@|
testAddress(person.address).validation.toValidationNel |@|
testPhone(person.phone).validation.toValidationNel
)(Person).leftMap(_.list).toEither
Although of course this is more verbose than necessary and is throwing away some information, and using Validation
throughout would be a little cleaner.