gojuno / koptional

Minimalistic Optional type for Kotlin that tries to fit its null-safe type system as smooth as possible.
Apache License 2.0
289 stars 21 forks source link

toNullable() specialization. #21

Closed artem-zinnatullin closed 6 years ago

artem-zinnatullin commented 6 years ago

Recent PR #20 made me rethink toNullable(), I think it needs to be abstract with specialized implementation in Some an None.

That gives following benefits:

Note that because of nullability change in Some.toNullable() the change is API incompatible, but ABI compatible so we're still allowed to release it in a minor update.

cc @Egorand, @AlecStrong

AlecKazakova commented 6 years ago

interesting...i wonder if this would mean component1 on None would have proper type Nothing? without having to make component1 abstract

artem-zinnatullin commented 6 years ago

Without abstract component1 None won't have component1 on its own, it's not a data class right

With abstract component1 however we're getting proper static nullability in both Some and None because it's declared as nullable in Optional so it works well for None and then compiler adds @NotNull to Some's override so it sees it statically as non-null. I mentioned that in https://github.com/gojuno/koptional/pull/20#discussion_r189455048 but maybe wasn't clear enough :)

artem-zinnatullin commented 6 years ago

As for component1(), I don't think it makes sense in None, so it should be specific to Some.

But #20 adds component1 to None (we can continue this discussion there)

artem-zinnatullin commented 6 years ago

@ming13 @dmitry-novikov @nostra13 can you PTAL ыы?