kitlangton / neotype

A friendly newtype library for Scala 3
Apache License 2.0
158 stars 16 forks source link

Errors should not be hardcoded to String #79

Open arturaz opened 2 months ago

arturaz commented 2 months ago

I was using Neotype and wondered what you think about the idea of having validate return any type rather than string?

So that make would return Either[ErrType, NewType] instead of Either[String, NewType].

The motivation is localization. Of course you can take a string and look it up, but it would be much more typesafe to have an enum there.

Obviously you still need a Show-like instance to turn that into a String for macro applications, something like:

given neotype.RenderToString[ErrType] = ???
kitlangton commented 2 months ago

Hey! Firstly, thanks for the issue 😊

The upside is quite clear; I'm generally a huge fan of typed errors. However, the design challenge would be how we could add this feature without complicating the definition of Newtype.

object PositiveInt extends Newtype.WithCustomError[MyError, Int] {
  def validate(input: Int): Either[MyError, Int] = ???
}

Perhaps the default Newtype[A] could be Newtype.WithCustomError[String, A], and we can have this other slightly more verbose option for those who want a fully custom type. And then, yes, it would be important that it can be rendered as a String. Indeed, another type-class would do the trick. Also, as described here, we could change validate to return an Either[Error, A].

My only concern is whether or not this would be "worth" the complexity added to the library implementation, documentation, and usage. And by "worth," I'm in no way dismissing your use case, but rather, attempting to quantify how common this use case is versus the normal Stringly-typed error message use case. I like to think about the Pareto Principle when it comes to these things—especially if there is a workaround, albeit a somewhat ugly one, for the uncommon case (the ugly workaround being the "take a string and look it up" approach).

I hope these concerns make sense. I'm not saying we shouldn't do this, just that i want to fully consider the trade-offs 😊 Let me know what you think!

arturaz commented 2 months ago

Perhaps the default Newtype[A] could be Newtype.WithCustomError[String, A], and we can have this other slightly more verbose option for those who want a fully custom type.

I agree.

My only concern is whether or not this would be "worth" the complexity added to the library implementation, documentation, and usage.

From the usage standpoint nothing changes for people who do not use this. Implementation looks fairly simple to me.

I could take a stab at it, but I have literally 0 experience with macros. Not sure this has to do anything with them though, what do you think?

kitlangton commented 2 months ago

Hey! Yeah, give it a shot 🥳 That would be great

arturaz commented 3 weeks ago

Did some initial work: https://github.com/arturaz/neotype/commit/97af1c6759cafe8b34414b932e3882d1c3df8f62

Reached out to you on ZIO Discord: https://discordapp.com/channels/629491597070827530/1272928549061263471/1272928551401947218