Kotlin / KEEP

Kotlin Evolution and Enhancement Process
Apache License 2.0
3.3k stars 356 forks source link

Optionals #291

Closed kevin1e100 closed 1 year ago

kevin1e100 commented 2 years ago

proposal for java.util.Optional extensions (KT-50484)

fvasco commented 2 years ago

I never defined three distinct methods for unwrapping, in my experience Optional<T>.orNull() is enough.

fvasco commented 2 years ago

For completeness, we propose including the following alternatives to Optional's factory methods

I propose to reconsider this section, should we provide a dedicated constructor for this type, only for completeness? A standard optionalOf method can create confusion with other types of Optionals, like Guava's provided one. Kotlin does not provide a streamOf, completableFutureOf or a publisherOf. What should be the type of optionalOf(3), OptionalInt or Optional<Int>?

I think that Optional.of* is enough, using Java syntax with Java classes is not so strange for me, factory methods can be shifted in future advancements if required.

ilya-g commented 2 years ago

While the benefit of having getOrElse extension over orElseGet member with the same signature is clear from the KEEP, it can still cause user confusion when these two functions are presented to them in completion. Perhaps an IDE could softly nudge to using getOrElse by explaining its advantages.

fvasco commented 2 years ago

The benefit of .getOrElse { } over the idiomatic .orNull() ?: should be explained, too. Two ways for the same thing.

r4zzz4k commented 2 years ago

@fvasco You probably mean .orNull() ?: run { ... }, which I would avoid when possible. Though being obvious to seasoned kotliners, it may still add enough cognitive load if used in complex expression. .getOrElse { ... } conveys the intention in a clean way, so I don't really see what there need to be explained by the IDE.

fvasco commented 2 years ago

@r4zzz4k I agree with your concern, but I don't see a significant difference in your example.

I confirm that I mean what I wrote.

r4zzz4k commented 2 years ago

Sorry, I re-read your message and understood it. Got you wrong on first reading. Yeah, maybe IDE helping to learn that would be a great idea!

kevin1e100 commented 2 years ago

Re: getOrXxx variants, the intention here isn't necessarily to provide a minimal set, but rather, a set of variants comparable to what Map for instance provides. It's not clear to me that being minimal is a goal here: if we're going to add extensions for Optional then IMHO we might as well add a robust set.

Re: optionalOf I agree that the case for including them is possibly a bit weaker (but see the text for some nuances). Personally, though, I again feel that we might as well introduce convenience for constructing Optionals, not least for symmetry, alongside extensions for unwrapping and transforming them. Other arguments are mentioned in the text, including again matching users' expectations by matching what's available for collections and elsewhere.

Re: optionalOf(4), point taken that it could be confusing, but seemingly that's also true for Optional.of(4). If we wanted to provide extensions for primitive wrappers to match what's proposed here, I'd imagine we'd introduce optional{Int,Long,Double}Of functions.

fvasco commented 2 years ago

@kevin1e100

getOrXxx variants, the intention here isn't necessarily to provide a minimal set, but rather, a set of variants comparable to what Map for instance provides

A Map is more complex than an Optional. A map can be empty, it can contain a value for a key and that value can be null or not. Instead, an Optional can be empty or not.

In Java, an Optional isn't similar to a Map, so we should argue why to coerce developers to consider an Optional to a Map. Some developers consider an Optional like a Collection with no more than one element. I consider an Optional like a hard Reference, or an object pointer, so adding Map's methods do not help me to develop with Optional, but force me to learn more things to develop with Optional.

I consider the only Optional's miss for a Kotlin developer orNull as the abbreviation of orElse(null).

I'd imagine we'd introduce optional{Int,Long,Double}Of functions

If there is a good reason to introduce the optionalOf builder, the same reason is applicable for all other Optional types. I don't see a good reason to introduce another builder, because Optional.of and others already exist.

joffrey-bion commented 2 years ago

While the goal of such addition as stated in the KEEP would be to help with Java interop, I'm afraid providing this many helpers may encourage the use of Optional within Kotlin code - where nullable types are more idiomatic and benefit from nice and concise operators (and declarations).

Like @fvasco, I believe Kotlin only really needs .orNull(). This function is useful because the vast majority of interop with Java's Optional would be to convert Optional return values of Java functions into nullable Kotlin types, which simply transforms the Java signature into what you would have had from the Kotlin equivalent. Considerations of getOrElse etc. shouldn't really apply because in Kotlin, you would have received a nullable value in the first place, so after .orNull() conversion one can just write idiomatic Kotlin code.

Optional arguments in Java methods are quite rare in comparison to optional return values (there are arguments for avoiding such practice), so I see little benefit in providing factory methods for those, especially since there are existing static factory methods on Optional that do the job quite OK. IMO this part is the one that would encourage the most to create more Optional values in Kotlin, which I believe would be a step backwards.

Also, Optionals are not supposed to represent collections, so the proposed conversion methods seem semantically debatable. If we consider that we unwrap optionals from Java methods into nullable values, this is the end of the interaction with the Java API surface. We can then use nullable values as if they were returned by Kotlin methods - using other helpers such as listOfNotNull etc.

bjmi commented 1 year ago

I never defined three distinct methods for unwrapping, in my experience Optional<T>.orNull() is enough.

Right, just KEEP it simple. Optional<T>.value and .value ?: resp. feels even more natural to me.

ilya-g commented 1 year ago

I'm going to merge this PR text and open new issue for the discussion of this KEEP: #321