Monadic result model

[buildreactive]

Thought I'd try a quick prototype. Ended up with this simple specs2 test:

val x = validate(new GPPersonInformation(GPID(1), simpleRandomName(), None, simpleRandomName(), Option("L")))

logger.trace(s"${x.getClass}")

if (x == com.wix.accord.Failure) {
    logger.error("FAILED")
}

Compiler reports:

[warn] /Users/zbeckman/Projects/Glimpulse/Server/project/glimpulse-server/test/models/TestPersonInformationDataModel.scala:62: com.wix.accord.Result and com.wix.accord.Failure.type are unrelated: they will most likely never compare equal [warn] if (x == com.wix.accord.Failure) { [warn] ^ [warn] one warning found

Also, you can run into name collisions with Scala Try / Success / Failure.

You might consider:

1. Use something like Validation / ValidationSuccess / ValidationFailure.

2. Add .isSuccess() and .isFailure() methods to make it easier to determine what is a success or failure. It seems we are required to write tests such as:

   if (x.isInstanceOf[Failure]) { logger.error("FAILED") }

Which is just plain ugly and hard to work with.

[packux]

A different approach would be to implement pattern matching for evaluating the output of validation i.e.

validate(GPPersonInformation(GPID(1), simpleRandomName(), None, simpleRandomName(), Option("L"))) match {
  case Success => //do sth on success
  case Failure => //do sth on failure
}

On name collisions, you may create aliases (if needed) when importing the specific accord types e.g.

import com.wix.accord.{Failure => ValidationFailure, Success => ValidationSuccess}

validate(GPPersonInformation(GPID(1), simpleRandomName(), None, simpleRandomName(), Option("L"))) match {
  case ValidationSuccess => //do sth on success
  case ValidationFailure => //do sth on failure
}

[buildreactive]

All very good points (I had forgotten Scala can do aliases). True, we can use pattern matching, although we generally avoid it when there is a simple, single case e.g., "if (success) {...}" as it is simpler to read.

[packux]

Continuing on this discussion, I think it would be very handy if the Result type would work in a similar fashion to the Try type of scala, i.e. provide map/flatMap facilities. That, would allow a syntax like the following

for {
   validatedDTO <- validate(DTOToValidate)
} yield validatedDTO 

or similarly

validate(DTOToValidate).map(validatedDTO => someDAO.insert(validatedDTO))

@holograph: Keeping in mind that this is not a small refactoring, would such a change make sense?

[buildreactive]

+1. We have a few data types that mimic Try and it works nicely (for instance, a ProcedureResult that wraps stored procedure calls).

It's also nice for building cascading validation rules, so if any one of the rules fails the whole thing gets kicked out. For now we are doing this (or, using matching, but I don't like to use match when a single if does the trick equally well):

    val v = Try(validate(device))
        .flatMap(v => Try(validate(identity)))
        .flatMap(v => Try(validate(credential)))

    if (v.isSuccess && v.getOrElse(ValidationFailure) != ValidationSuccess) { ... }

Anything beyond v.isSuccess is probably unnecessary. I just code paranoid...

[holograph]

@zbeckman I may be missing something but I don't see the point in having Try there; also, even with the current API you can do:

val v = validate(device) and validate(identity) and validate(credential)
if (v == com.wix.accord.Success) { ... } else { ... }

The difference from your original code is that Success is a singleton object, whereas a Failure is a case class which contains the violations. You'll need to either use v.isInstanceOf[Failure] or pattern match.

@packux Since Accord operates, as an intentional design goal, on pre-initialized objects (i.e. after deserialization, form input, database load etc.), it can't really offer scalaz-like "applicative builder" object construction. That is the obvious use case for a monadic result model.

On the other hand, it's entirely possible that I missed your point -- is validatedDTO simply the object passed to the validator to begin with?

[packux]

@holograph : the idea is to provide a facility similar to the Try monad. That would allow an easy way to execute further functions on the value in case of Success or simply return a Failure.

For example, a map function would allow the following

case class Foo (foo: Int, bar: Int)

object Foo {
  implicit val fooValidator = validator[Foo] { f =>
       f.foo > 0
       f.bar > 0
  }
  impicit val restFormat = Json.restFormat[Foo]
}

object FooApp extends Controller {
   implicit val fooRestFormat = Foo.restFormat

  def createFoo = Action { request => 
    for {
       foo <- request.body.validate[Foo]
       validatedFoo <- validate(foo)
       fooID = fooDAO.insert(validatedFoo) // we assume this returns an Int
    } yield fooID

  }

}

In a similar fashion, flatMap would allow some nested validations for objects that are not available when we validate the initial object, for example

case class Foo (foo: Int, bar: Int)

object Foo {
  implicit val fooValidator = validator[Foo] { f =>
       f.foo > 0
       f.bar > 0
  }
  impicit val restFormat = Json.restFormat[Foo]
}

case class Bar (fooID: Int, bazz: Int)

object Bar {
  implicit val barValidator = validator[Bar] { b =>
       f.fooID > 0
       f.bazz > 0
  }
  impicit val restFormat = Json.restFormat[Bar]
}

object FooApp extends Controller {
   implicit val fooRestFormat = Foo.restFormat

  def createFoo = Action { request =>
   for {
       foo <- request.body.validate[Foo]       
       validatedFoo <- validate(foo)
       fooID <- fooDAO.insert(validatedFoo) // we assume this would return an Option[Int]
       validatedBar <- validate(Bar(fooID = fooID, bazz = 1)
       validBarID <- barDAO.insert(validBar) //we assume this would also return an Option[Int]
    } yield validBarID
  }
}

[holograph]

Well that's the thing -- what does request.body.validate[Foo] do? It both deserializes and validates, and the deserialization aspect is completely outside of Accord's purview...

[voidcontext]

+1 for the monadic features, it would be more idiomatic and easier to integrate.

[holograph]

I'm still missing some basic notion of what these wrap. Taking the previous example:

  def createFoo = Action { request =>
   for {
       foo <- request.body.validate[Foo]       
       validatedFoo <- validate(foo)
       fooID <- fooDAO.insert(validatedFoo) // we assume this would return an Option[Int]
       validatedBar <- validate(Bar(fooID = fooID, bazz = 1)
       validBarID <- barDAO.insert(validBar) //we assume this would also return an Option[Int]
    } yield validBarID
  }

There are too many clauses and types here; for example, why is foo seemingly validated twice? What is the type of validatedFoo? Also, you seem to be mixing monads (form validation, Accord validation and Option) without the appropriate transformers, so I'm not sure this could ever be made to work :-)

Given Accord's design decision to only validate complete objects (as opposed to scalaz's "applicative builder"-style iterative construction), the only sensible behavior I can think of is a "passthrough" i.e.:

val rawFoo: Foo = // ...
for (validatedFoo <- validate(rawFoo))   // validatedFoo is of type com.wix.accord.Success[Foo]
  yield validatedFoo.bar                 // Result is of type com.wix.accord.Success[Bar]

With a validation result being inherently success-biased (i.e. failures fall through, again similar to scalaz's disjunction). This feels to me to be of very limited value, but perhaps you can provide some additional examples of where this would be useful.

If you want to weigh in on this, the most useful comment would be "here's a piece of actual production code; here's what I would want it to look after refactoring", and don't forget to annotate the types so it's more obvious what you're looking for :-)