https://kotlinlang.org logo
Title
a

arekolek

03/04/2020, 11:21 AM
https://blog.jetbrains.com/kotlin/2020/03/kotlin-1-3-70-released/
You know this convention in Kotlin: having a pair of functions, where the first one throws an exception if the operation isn’t possible, and the second one returns 
null
, like 
string.toInt()
 and 
string.toIntOrNull()
. Now we’ve added new 
randomOrNull()
 and 
reduceOrNull()
 counterpart functions, following the same convention
If this is a convention, then why there is no
max(): T
and
maxOrNull(): T?
and instead we have a
max(): T?
? We had a discussion about it here https://kotlinlang.slack.com/archives/C1H43FDRB/p1551864308014200
i

ilya.gorbunov

03/04/2020, 11:38 AM
max()
was developed prior to the point when that convention has emerged. But we're considering now, how to bring some consistency here too.
👍 5
s

spand

03/04/2020, 11:43 AM
If the convention is strong enough then it might be relevant to consider additional syntax ie.
random!() // previously random
random?() // previously randomOrNull
to fall in line with
!!
being the throwing variant.
💯 1
👀 1
🤔 2
a

arekolek

03/04/2020, 1:35 PM
Maybe: 1. Deprecate
max(): T?
in favor of
maxOrNull(): T?
in some future Kotlin version. 2. Remove
max(): T?
in some later version. 3. Then introduce
max(): T
in some later version?
l

louiscad

03/04/2020, 6:00 PM
And allow to switch immediately with progressive mode?
👍 1
i

ilya.gorbunov

03/05/2020, 3:56 AM
The initial plan was something like: - introduce
maxOrNull(): T?
in the same package - introduce
max(): T
in a separate non-imported by default package - then the current
max()
can be replaced with
maxOrNull()
, and
max()!!
can be replaced with the new
max()
from another package with an explicit import. - after the original
max()
goes through a deprecation cycle, we can "move" new
max()
in place of the old one with another deprecation cycle. The last migration could be perfectly automatable, because it would just require to remote the import. However, given the current state of completion and auto-import it seems too brittle to have a function with the same in another package that behaves differently: an occasional import can change behavior of all invocations in the file.
d

Derek Peirce

03/05/2020, 6:44 AM
Personally, I think that the convention of having only the nullable method being labeled properly is a mistake, perhaps the largest mistake in the standard library. Using a method like
first()
or
random()
just entirely hides the possibility of failure due to an empty list until runtime hits, which I've seen happen many times. It's the same design problem as nulls. I would prefer if the default were nullable and the error-able ones were something like
firstOrThrow()
, or if everyone used
first()!!
but by some new language feature, the method
first
includes instructions for how a null result should be interpreted, so that
!!
called on its result gives a proper error message instead of an NPE (just requiring some other workaround for when the list itself has nullables, and the first value could just be null), so that we wouldn't have to double the API.
2
s

spand

03/05/2020, 7:43 AM
@ilya.gorbunov Is that the plan for
max()
specifically or in general ? Because it seems pretty analogous to most(all?)
x()
and
xOrNull()
pairs in the entire stdlib
i

ilya.gorbunov

03/05/2020, 11:37 AM
@spand What other
x
functions do you have in mind?
s

spand

03/05/2020, 12:00 PM
On iterable/collection/list there are at least
first
,
last
,
single
,
random
,
elementAt
. On String theres
get
,
toInt
,
toShort
...
i

ilya.gorbunov

03/05/2020, 1:26 PM
These ones do not have such problem with nullability as
max
and all have
...OrNull
variant
s

spand

03/05/2020, 2:00 PM
Ah sorry. I was on a different train of though here
a

Astronaut4449

03/05/2020, 3:30 PM
@Derek Peirce Yes, it does hide the possibility of an error but in my opinion this should be the responsibility of the developer. I think it's not that critcal though, because whenever you type
first
in your IDE you will immediately get the suggestion
firstOrNull
as well which will remind you of the possibility of having an empty collection. I am much in favour of
first
throwing an exception rather than returning null because: 1.
first()!!
doesn't read well 2.
first()
returning null doesn't tell you wether the first item was null or no item was found in case the collection contains a nullable type. I would also love
Map.get()
to throw, but I know it's impossible due to interoperablility with Java. Writing
map.getValue(key)
instead of
map[key]
hurts every single time.
d

Derek Peirce

03/06/2020, 5:43 AM
@Astronaut4449 it appears that we're on opposite sides with respect to the stdlib and nullability. I'm very much on the side of defensive programming, to the point where I'm suspicious of even accessing a raw list index due to the potential
IndexOutOfBoundsException
, instead of using something like
firstOrNull
. (Ideally, we'd have some annotation to denote a list that's proven to be non-empty, and methods that can only operate on such lists, but that's beyond today's Kotlin.) When I see a call to
first()
in a code review, I can't tell if the author has guaranteed elsewhere that the call is safe, or actually meant
firstOrNull()
. (It's often the latter.) On large projects, the IDE is often still indexing while I'm typing my code, so it isn't always a defense. For the problem of the collection containing nulls, perhaps the solution is to have all of the methods have a signature of
firstOr(ifEmpty: () -> T)
, which could then be easily delegated to for
firstOrNull()
and
firstOrThrow()
. Perhaps the names could be improved, but I'm generally against methods having the reasonable possibility of throwing an exception without that being clear from the name.
a

Astronaut4449

03/06/2020, 8:09 AM
@Derek Peirce I think there is no right or wrong. It comes down to personal preference. Whenever I access an element in a collection via
get(index)
it is clear to me that it might throw an exception and that as a developer I must ensure that this item exists.
first()
is just the same as
get(0)
and therefore has the same behaviour. Consequently
first { ... }
throws an exception as well. To be consistent throughout the language all of these operations should rather throw by default and offer a null function as alternative. One could argue that
max()
is different though.
max()
cannot be called on a collection of nullable types. So the there is no ambiguity between null as valid return and null as failure indication. Still I prefer to have one convention without too many exceptions. Btw in an ideal world maybe contracts could tell the compiler that the list is not empty when
isNotEmpty()
has been called and therefore methods like
first()
,
reduce()
, etc cannot throw/return null.
d

Derek Peirce

03/08/2020, 2:28 AM
get
is a bit strange in that it is a safe call on a map, but not on a list. I prefer when all assumptions must be made explicit, and can't be made by accident, but that might be just me. As for the ideal world, I would imagine an annotation like
@NotEmpty
that works like
@NotNull
works in the Java Checker Framework: • By contract,
isNotEmpty
implies that an immutable value can be treated as annotated with
@NotEmpty
• Many extension methods exist for iterables/strings/etc. that can only be called if it is annotated with
@NotEmpty
, something like
fun @NonEmpty String.first(): Character
• Similar to the Checker Framework, the annotation works within generics, and follows the covariance rule of
@NonEmpty T : T
I work with many lists that constantly have assertions to guarantee that they have at least one item, it would be nice for the compiler to ensure that instead.
i

ilya.gorbunov

05/20/2020, 8:53 PM
We have submitted the following proposal to the language committee to gradually change the return type of
min
and
max
operations: https://youtrack.jetbrains.com/issue/KT-38854
💯 4
:kotlin-flag: 1
👍 9
🙌 1
a

arekolek

10/15/2020, 9:34 AM
Looking back at this plan for `min`/`max` related functions:
1.4: introduce "...OrNull" functions as a synonyms, KT-39064
1.4: deprecate the affected API with the proposed replacement to the "...OrNull` variants above, KT-39064
1.5: raise the deprecation level to error
1.6: reintroduce the affected API, but with non-nullable return type
I wish it had in addition these steps: • 1.4: introduce "...OrThrow" functions in preparation for non-nullable variants • 1.6: deprecate "...OrThrow" with proposed replacement to the reintroduced APIs with non-nullable return type • 1.7 raise the deprecation level to error
2
2
i

ilya.gorbunov

10/15/2020, 1:55 PM
We'd rather avoid introducing non-experimental API that is going to be retracted two releases later.
a

arekolek

10/15/2020, 4:55 PM
fair enough 🖖