Shouldn't ``` public operator fun <T> Iterab...
# stdlib
m
Shouldn't
Copy code
public operator fun <T> Iterable<T>.minus(element: T): List<T>
be defined with element as nullable?
Copy code
public operator fun <T> Iterable<T>.minus(element: T?): List<T>
I.e. if you call
listOf<String>(...) - null as String?
it would just return the same
List<String>
and not
List<String?>
as it currently does.
3
Anyone has any input on this idea? I think such a change could be implemented without breaking any code using
minus
currently. It could even be changed to
element: Any?
, but I'm not sure this would be very useful.
s
what if
T
nullable itself?
m
There would be no problem. It would work exactly the same.
T
is subclass of
T?
s
listOf(1, null, 2) - null
this would work?
m
listOf(1, null, 2) - null as Int?
s
For now, operator works like this:
Copy code
println(listOf(1, null, 2) - null as Int?)
outputs: 1, 2
if you change operator to something like this:
Copy code
public operator fun <T> Iterable<T>.minus(element: T?): List<T> {
    if (element == null) {
        return this.toList()
    }
...
then
Copy code
println(listOf(1, null, 2) - null as Int?)
output: 1, null, 2
or I'm missed something?
m
Copy code
val list1: List<Int?> = arrayListOf(1, null).minusNullable(null)
        println("list1: $list1")
        val list2: List<Int> = arrayListOf(1, 2).minusNullable(null)
        println("list2: $list2")
compiles and prints:
Copy code
list1: [1]
list2: [1, 2]
where
minusNullable
is a slightly changed
minus
operator (essentially argument type changed to
T?
):
Copy code
fun <T> Iterable<T>.minusNullable(element: T?): List<T> {
    val result = ArrayList<T>()
    var removed = false
    return this.filterTo(result) { if (!removed && it == element) { removed = true; false } else true }
}
👍 1
@snrostov
if (element == null) { return this.toList() }
is wrong. If it's
null
, you still want to remove it if it exists in the list.
Good thing about changing argument type to
T?
is that it still returns
List<T>
even if you pass something that is potentially null.
s
I see now, thanks for explanation
m
And as
T?
is supertype of
T
(as well as
Any
and
Any?
are), no code existing currently would break.
i
This has to be investigated, whether it's possible to add something like this: https://github.com/JetBrains/kotlin/blob/1.1.0/libraries/stdlib/src/kotlin/collections/MutableCollections.kt#L15 so that type of element to be removed could be any direct supertype of element type of the collection, and not only T?.
@mg6maciej Please open an issue
m
Thanks. I will.
Although I'm mostly interested in a change to
T?
, but
Any?
could possibly also be used instead.
@ilya.gorbunov The more I use Kotlin in a "immutable only" way, the more I think there is a need to a good immutable collections library 😉
So I wasn't really considering change to
MutableCollection::remove
, just to
Iterable::minus
(with element, as well as other arguments).
So I wanted to make this change in Kotlin's stdlib and see how it works, but I failed to run all tests. Is there a "tutorial" what needs to be done to run stdlib's test suite? I finished pretty early at setting JDK_16, 17 and 18. @ilya.gorbunov
@ilya.gorbunov It's not only for direct supertype, unless direct means something else.
Copy code
open class A
open class B : A()
class C : B()

fun usageTest() {
    val list = mutableListOf<C>()
    list.remove(null as Any?)
    list.remove(Any())
    list.remove(A())
    list.remove(B())
    list.remove(null as C?)
    list.remove(C())
}
All compile.
Interesting tho that it is implemented for mutable collections like I would like it to behave for "immutable" ones.
i
It's not only for direct supertype, unless direct means something else.
Yeah, turns out I've actually meant something else 😆 I mean you shouldn't be able to minus a
String
from a
List<Int>
even if you can minus
Any
from that list. That's how
MutableCollection.remove
extension works. Although now you can, so it might be a source breaking change.
So I wanted to make this change in Kotlin's stdlib and see how it works, but I failed to run all tests. Is there a "tutorial" what needs to be done to run stdlib's test suite? I finished pretty early at setting JDK_16, 17 and 18.
Yes, take a look at this readme: https://github.com/JetBrains/kotlin/tree/master/libraries If you have any problems with building and running tests, welcome to share them in #kontributors
👍 1
m
"Although now you can, so it might be a source breaking change." Oh, true. It returns
List<Any>
in that situation. That's a shame.