-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
ValidatedNel and NonEmptyList are invariant #1419
Comments
Yeah, I faced it, and it's quite confusing. |
+1 to covariant data types where possible. |
Mmm there might be good reason to keep it invariant. I don't have details, but off the top of my head the fact that you can easily get into a situation of inferring |
Yeah, all possible implications must be taken into account before making any decision (whether to make it covariant or not), but for me invariant version looks a little bit counterintuitive. First of all, as @edmundnoble said, List and Validated are covariant, and NonEmptyList is positioned as just a version of a List with a guarantee to have at least one element in it. So, I think, it is quite logical to expect it it be covariant. Also, Scalaz's NonEmptyList implementation is covariant too, and many users transition to cats from that land. It is not the final argument in any sense, it's perfectly fine for two libraries to have such differences, but it is a case that should be considered too. Also, I'm curios, what Scalaz guys think about it, what was their motivation behind this design decision? And after all, at a logical level, forgetting about concrete language implications, the collection of bikes is for sure should be able to be treated as a collection of vehicles. Just a dead simple logic behind all this variance stuff. (Yes, I know, it technically may be using widen[], but we must be sure that we exchange extra boilerplate to something that worth it) |
@adelbertc I believe the widen-to-Any issue is mostly an issue with putting ops on the data classes themselves and not enriching them on; you can avoid widening in every method's type args if your ops class isn't covariant. That would be a binary-incompatible change, but it would be safer. I believe cats should probably have a policy on this first. |
Added in #1424 |
List
andValidated
are covariant, butValidatedNel
andNonEmptyList
are not.The text was updated successfully, but these errors were encountered: