oolong-kt / oolong

MVU for Kotlin Multiplatform
http://oolong-kt.org
Apache License 2.0
298 stars 14 forks source link

Remove non-null type bounds. #115

Closed pardom closed 4 years ago

jcornaz commented 4 years ago

That looks good thanks.

To quickly answer to your message in https://github.com/oolong-kt/oolong/issues/108#issuecomment-683310025

You are free to use Maybe for Model, Msg, and Props in Oolong as well.

Well, sure. But Maybe is not part of the kotlin standard library. And it will probably be never added, since it would be ambiguous with type nullability. When to use T? instead of Maybe<T>? What would Maybe<T>? mean?

However, thanks for creating this PR. ;-)

pardom commented 4 years ago

When to use T? instead of Maybe? What would Maybe? mean?

You shouldn't, and that's exactly my point. The constraint was in place to enforce functional modeling of absent values. Removing it will make Oolong more approachable for newcomers and has no impact if you prefer Maybe, like I do.

jcornaz commented 4 years ago

But Maybe does not exist in Kotlin. Does it?

The constraint was in place to enforce functional modeling of absent values

We already have that in Kotlin via type nullabilty. See my example in https://github.com/oolong-kt/oolong/issues/108#issuecomment-683295015.

The fact that it is using null does not make it less "functional". In my example, there is no side-effect and every function (both defined and used) are pure.

torresmi commented 4 years ago

I definitely prefer nullable types over maybe/option monads, but I think the focus should be on if any optionality makes sense in these APIs.

I can see a case for Model and Props if one doesn't want a wrapper class. I don't see it being helpful to restrict those.

Messages are where it becomes trickier for me. What is the intent for returning null/Nothing as a message in Oolong APIs? That update shouldn't run at all? Or it does run and handles the logic like normal? If that causes confusion then I can see it helpful not allowing null there to prevent any misleading assumptions. Sure one could provide a user specified type like Maybe, but I think most wouldn't have the assumption that their Nothing type would change runtime behavior.

Any thoughts on just leaving it for messages?

pardom commented 4 years ago

Maybe does not exist in the language or in the standard library. Typically you would import Arrow's Option or something similar. If you use Arrow's Option type, it has an instance for many typeclasses.

I agree with @torresmi that the focus should be on whether nullability makes sense in these APIs. In this respect, I think we should prefer consistency and choose to be either fully restrictive or fully unrestrictive. If we are giving users the freedom to represent Model and Props as null then I think we should entrust them with the same freedom for Msg. In Elm, you can simulate a null Msg by having a NoOp instance for msg. I recommend we allow all types to be nullable and offer guidance in the docs.

torresmi commented 4 years ago

Sounds good to me 👍