-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Custom Checked monad #10
Comments
I've toyed with the idea of making a type class generalizing over typelevel/cats#976 for reference. |
I've actually just bumped up against this exact problem. I wonder if Would it be fair to say that type Checked[T] = NonEmptyList[WarningMessage] Ior Validated[NonEmptyList[ErrorMessage], T] I think the above problem above stems from the fact that, by smooshing the hard and soft failures together, you can't really enforce at the type level whether the messages on the left are a hard failure (therefore throw the value By separating these out you can keep the existing monadic behaviour of val foo: Checked[T] = ...
val bar = foo map { _ map { t: T => ... }} since we need to |
Actually, a couple of other nice benefits fall out of this: it's no longer necessary to have separate This then makes it trivial to implement custom message types (#2): type CheckedCustom[M, T] = NonEmptyList[M] Ior Validated[NonEmptyList[M], T]
// Obviously Message is a bit more complex than this...
case class Message(text: String, path: Path)
type Checked[T] = CheckedCustom[Message, T] (Although you'd need some way of passing in a converter |
@ashleymercer Your observation is mind-bogglingly obvious in hindsight. I think this is a great idea. |
I think it would be good for this to be it's own type, with conveniences for converting in to an Ior/Validated composition. As it's own type, it'll be less cumbersome and we'll be able to define semigroups/monad instances without much hassle from overlap. sealed trait Checked[E, W, A]
case class Errored[E, W, A](errorMessages: NonEmptyList[E], warnings: List[W]) extends Checked[E, W, A]
case class Warned[E, W, A](warnings: NonEmptyList[W], result: A) extends Checked[E, W, A]
case class Succeeded[E, W, A](result: A) extends Checked[E, W, A] Maybe throw in some variance to get rid of some type params, maybe not. From here you just build typeclass instances to behave how you'd expect an A Ior (Validated[B, C]) to behave. Maybe E and W should be unified, but I think it's useful to separate them out. Might have to make a trifunctor and tritraversable to make them easier to work with. |
If we do wind up with trifunctor, tritraverse, then #3 will require us to add a module for scalaz support, since shims will not cover our custom typeclasses. |
Now that I think about it more, you're right - removing the risk of clashing with existing typeclasses is very useful, and we can easily map back to the "vanilla" cats types if needed.
Again, I agree - the obvious example is that you're going to want your error type to hold some exception or stack trace, but the warning type is probably just a String.
I think at least |
Abstractions on abstractions: had a bit more of a think about this and maybe it makes sense to introduce a type Trior[A, B, C] =:= Either[Either[A, (A, B)], Either[(B, C), C]]
type Checked[T] = Trior[NonEmptyList[Message], NonEmptyList[Message], T] |
@ashleymercer There's actually a ton of these isomorphisms, I'm discovering. def eithers: Either[Either[E, (E, W)], Either[(W, R), R]] =
fold(
(e, w) => w.fold(Left(Left(e))(Left(Right((e, w))))),
(w, result) => Right(Left((w, result)))
result => Right(Right(result))
)
def eitherOption: (Option[W], Either[E, R]) =
fold(
(err, warn) => (warn, Left(err)),
(warn, result) => (Some(warn), Right(result)),
result => (None, result)
)
// this one isn't an iso, but whatever. it could be nice
def options: (Option[E], Option[W], Option[R]) =
fold(
(err, warn) => (Some(err), Some(warn), None),
(warn, result) => (None, Some(warn), Some(result)),
result => (None, None, Some(result))
) |
I spent a little time on an alternative encoding of In this codebase, the I'm interested in trying different encodings, for example on top of @ashleymercer's trior type above, to see if they create useful alternative sets of semantics. |
Revisited this again (!) and realised there's an alternative, even simpler encoding we might use: sealed trait Checked[E, W, A]
case class Failed[E, W](errors: E, warnings: Option[W]) extends Checked[E, W, Nothing]
case class Succeeded[W, A](warnings: Option[W], value: A) extends Checked[Nothing, W, A] This encoding has several advantages:
On the last point: with a (very simplified) checked match {
case Ior.Left(errors) =>
errors.foreach(log.error)
System.exit(1)
case Ior.Both(warnings, value) =>
warnings.foreach(log.warn)
businessLogic(value)
case Ior.Right(value) =>
businessLogic(value)
} so you end up repeating business logic in both branches (or else factoring out to a separate method, but you still have to call it in both branches). With a simple two-case match it looks much cleaner: checked match {
case Failed(errors, warnings) =>
errors.foreach(log.error)
warnings.foreach(_ foreach log.warn)
Sys.exit(1)
case Succeeded(warnings, value) =>
warnings.foreach(_ foreach log.warn)
businessLogic(value)
} I've only got a very preliminary prototype so far, will do a bit more digging and maybe throw up a gist or something. |
Aaand of course I've hit an immediate snag: using Asking people to use a custom case class Failed[E, W](errors: E, warnings: W) extends Checked[E, W, Nothing]
case class Succeeded[W, A](warnings: W, value: A) extends Checked[Nothing, W, A] with the caveat that they should use a possibly-empty-but-accumulating type like |
Hi! Thanks for working on this. My first inclination (without giving it too much thought) is along the same lines. The types should be just And I like your encoding with |
If you do SemigroupK[Option] doesn't it behave more like first one set? Aka it doesn't rely on W itself being a monoid. |
Here's a very rough-and-ready implementation: I haven't remotely touched any of the As desired, warnings are generally preserved as long as possible; errors obviously can't be (m)ap'ped over, but can be combined. Since Type inference seems to work pretty well (I've followed the rough layout of cats for implicit resolution etc) but since we only have two case classes and the warning type NonEmptyList.one("This is a fatal error").fail[List[WarningType]] Alternatively, you can specify the type of the final result, and let the compiler work downwards: val res: Checked[NonEmptyList[String], List[String], Foo] = ... I'm not sure there's much we can do to avoid this tax. |
I think we should change the default semantics of
Checked
. This thread is intended for that discussion. To provide more detail...The semantics of
Ior
allow the user to map on the right hand side, regardless of what values are on the left hand side. Most rules in checklist allow the user to signal errors and retain a value on the right. This gives slightly odd semantics where error handling doesn't fail fast when the programmer expects it to.@Jacoby6000 provided a workaround for this in #9 by adding "strict mode" checks that return an
Ior.left
on failure instead of anIor.both
. However, I still think this is too fiddly. IMO a developer should be able to choose failure semantics up front and not have to worry about them afterwards. See #2 for thoughts on allowing abstraction of theChecked
type.Regardless of whether we can abstract over different implementations of
Checked
, we still need to choose a default failure semantics. These are the properties I believe most developers (myself included) will expect:map
andflatMap
;field
andfieldWith
(and possibly
ap
andmapN
);The problem with this is that
flatMap
andap
have inconsistent semantics. This is something that Cats and Scalaz fundamentally avoid. However, I believe that in the case of checklist, a non-rules-compliant customChecked
monad will be a reasonable thing to create.What do people think? Is this an ok idea? Is it an ok idea if we allow users to swap out
Checked
forEither
orIor
as suggested in #2?The text was updated successfully, but these errors were encountered: