https://kotlinlang.org logo
Join Slack
Powered by
Is there a nice way to define a list of extension ...
# announcements
n
Is there a nice way to define a list of extension functions? Background: in one of our user group discussions we discussed various ways to code "replace or add" for lists. We then wanted to test all of the proposed solutions using a `listOfProposals.forEach { f -> ... }`but could not find a way to define such a list. The best I have right now is to use a list of non-generic anonymous functions, where each item calls one of he proposed solutions. But that does not look "nice":
Copy code
fun <T> List<T>.replaceOrAdd1(item: T, predicate: (T) -> Boolean): List<T> { ... }
fun <T> List<T>.replaceOrAdd2(item: T, predicate: (T) -> Boolean): List<T> { ... }
data class Person(val name: String, val age: Int, val email: String, val address: String = "")
val listOfProposals = listOf(
    fun(list: List<Person>, person: Person, predicate: (Person) -> Boolean) = list.replaceOrAdd1(person, predicate),
    fun(list: List<Person>, person: Person, predicate: (Person) -> Boolean) = list.replaceOrAdd2(person, predicate),
)
// <https://pl.kotl.in/GvbQWnFEq> is the link to the working example
n
You can write
Copy code
val functions = listOf(
    List<Person>::replaceOrAdd1,
    List<Person>::replaceOrAdd2,
    List<Person>::replaceOrAdd3,
    List<Person>::replaceOrAdd4,
)
seems to compile without changing anything else
is that the improvement you are looking for? Not completely certain what the part is that you think isn't nice
I guess you are also concerned that it is not generic?
n
That was my first idea but does not compile for me. Which Kotlin version and backend?
y
Here's a neater version based on the code that you provided: https://pl.kotl.in/x6z-_JUPe
n
i just changed it at the link you provided me
and it compiles
Also, if you want to make it generic, simply make it a function instead of a variable
Copy code
inline fun <reified T> functions() = listOf(
    List<T>::replaceOrAdd1,
    List<T>::replaceOrAdd2,
    List<T>::replaceOrAdd3,
    List<T>::replaceOrAdd4,
)
y
It is basically what @Nir suggested but for some reason including the List type makes it compile
n
I get "replaceOrAdd1 is a member and and an extension at the same time. References to such elements are not allowed"
y
is replaceOrAdd1 inside of a different class? as in is it a member of a class?
n
n
hmm. that works for me as well. But my "scratch file in IntellJ" gave me that error I posted above.
"using the List type" was the missing part of my attempts. Perhaps I have too much trust in Kotlin type inference because I did not even try that. I also like the 
inline fun <reified T> functions()
approach
btw: which of the 4 implementations would get your vote? As a reference, our lists will at most contains a few dozen entries, so elegance and understanability rate a bit higher than performance for me here.
n
yeah, the inline approach is the only way to keep it generic, which IMHO is pretty useful, you may want to test on different types and there's no real downside
For readability + reasonable performance, I would say that 4 is probably the best
It's more efficient than 1, and about equally readable
2 is, IMHO, just bad
n
I just tried the solutions in my scratch file and I still get the same error. Full disclosure: the scratch file does not have the 
fun main()
wrapper so that I can execute the code. But that seems to have side effects. I just started using these scratch files and perhaps do not yet underdtand their limitations
n
2 is both harder to read, and it's actually pretty inefficient (fully copy the list twice if replacing does not occur)
4 is probably the most efficient, I think. But it would be a big difference.
not be a big difference sorry
Ugh sorry, 3 is the most efficient
n
#2: yes, looked a tiny bit better until it was realized that w/o the 
notReplaced
in the loop it would perform a "replaceAll" instead of "replaceFirst"
n
getting n umbers mixed up
basically, only 3 and 4 are even worth considering here
4 being a bit more readable and concise, 3 being a bit more efficient, I'd choose 4.
1 and 2 are dominated by other solutions
n
3 and 4 are essencially identical from performance POV, no?
n
not quite
n
I like 4 better just because it's easier to understand for me
n
yeah, 4 is the most readable for sure.
4 is two-pass
3 is, most likely, one pass
n
yes, agreed
n
but, I dunno, once the JVM is done with it, I'm on very thin ice to speculate which will be faster, if there will even be a difference... in C++ for a large vector (list), I'd be pretty confident that 3 would be fastest.
3 can be ensured to be faster by reserving enough space beforehand
if you use arrayList directly
n
yeah. but 3 really looks like "trying too hard" for me.
n
but we're really getting into the weeds for questionable benefit here. So I'd just stick to 4.
It's trying too hard for the level of benefit
n
🙂
n
mutable data structures are definitely necessary sometimes, I wouldn't allow something to become N^2 instead of N just to avoid a local mutable variable
but here's it's all just linear anyway, one extra pass, who cares
n
yup. thx for your feedback
n
👍
minor comment, if you supply 4 functions like that, I'd really suggest doing the 1-2-3-4 in written order
lol, I kept getting confused because my brain just assumed that, that's why I kept saying the wrong number before
e
I'd go for something like 4 but with 
buildList
n
yeah, should have cleaned it up a bit more.
was collected from a larger discussion chat
e
Copy code
fun <T> List<T>.replaceOrAdd(replacement: T, predicate: (T) -> Boolean): List<T> = buildList {
    val replaced = false
    for (item in this@replaceOrAdd) {
        if (!replaced && predicate(item)) {
            add(replacement)
            replaced = true
        } else {
            add(item)
        }
    }
    if (!replaced) {
        add(replacement)
    }
}
not the shortest, but to me I'd rather write imperative code than have a mix of mutability and functional code
n
needs a 
!
in the last if... never mind, you just corrected
n
eh, I dunno, to me it seems very natural to first establish whether you'll be replacing, or adding, as it leads to two very different approaches
e
actually does make me think of trickier solution though:
Copy code
fun <T> List<T>.replaceOrAdd(replacement: T, predicate: (T) -> Boolean): List<T> = buildList {
    val iterator = this@replaceOrAdd.iterator()
    for (item in iterator) {
        if (predicate(item)) break
        add(item)
    }
    add(replacement)
    for (item in iterator) add(item)
}
n
def. scores high on the "tricky" scale
but neat
n
if you like to work with list and creating new data structures a lot (as this code seems to indicate) then you may also want to add a 
replaceAt
function
List<T>.replaceAt(index: Int, value: T): List<T>
basically the non-mutating version of []
n
on of my team member s brought that up because he ran into such "replace or add" situation. Don't think it's very common
n
maybe not, but to be fair, this 
replaceOrAdd
function is arguably quite a bit more natural (and efficient) on a mutable list to start with
j
@ephemient I think that will always add the replacement even if the predicate returns false for every element. Not 100% if that's desirable as I only briefly skimmed the prior 66 messages. 🙂
n
yes, that is the idea: either replace (if matching), or else add at the end of the list. Kind of an "upsert"
👍 1
j
Copy code
fun <T> List<T>.replaceOrAdd(item: T, predicate: (T) -> Boolean): List<T> {
    val left = takeWhile { !predicate(it) }
    return left + item + (this - left).drop(1)
}
I submit my readable but untested slightly suboptimal version.
n
maybe I'm misreading minus but that seems incorrect
e
sure, inefficient one-liner: 
takeWhile { !predicate(it) } + item + dropWhile { !predicate(it) }.drop(1)
👆 1
n
I'm probably liable to steer clear of anything involving List - List
j
Yep that's what I was thinking. For optimal I like the 
buildList
version you proposed.
n
it seems like List - element and List - List even differ significantly in unexpected ways. List - element only removes the first occurence. List - List removes all occurences.
n
I read through these 
takeWhile
versions 3 times now and still can't confirm their correctness in my head. I e.g. wonder if this really implements the "only replace the first match" condition (which actually was never really specified, but all the other implementations do that)
e
mine does, @Joel’s doesn't
j
Where's the javadoc when you need one??
n
in the graveyard, with KDoc dancing on it's grave...
e
https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/minus.html have to look at the right overload... as @Nir says, the behavior is different between .minus(element) and .minus(elements)
n
IMHO overloading minus for List - element, is, borderline, the List - List one though is really not too cool
I can't recall ever seeing that in another language
j
Does List.remove take out duplicates?
n
You mean duplicates in the List, that aren't on the list of removals? I don't think so.
j
mutableListOf(1,2,3,2,1).remove(2)
e
well, it's that there isn't a particular overload for .minus(elements: List<T>), it's for .minus(elements: Iterable<T>)
j
Alright fine throw my solution in the trash, see if I care
It's suboptimal anyways 🤣
e
now, of course the latter could be implemented as
Copy code
fun <T> Iterable<T>.minus(elements: Iterable<T>) {
    val remainingElementsToRemove = elements.toMutableSet()
    return this.filterNot { remainingElementsToRemove.remove(it) }
}
and it would have the "remove only once" behavior
but it's not, it doesn't convert a Set to a MutableSet, so it doesn't keep track of how many times it has removed an element
thus, it removes all repetitions
.minus(element) = .remove(element), .minus(elements) = .removeAll(elements)
j
The default implementation of minus does not, correct?
I like that minus implementation, need to write that one down somewhere
e
@Joel not sure what you mean. 
listOf(1, 2, 3, 1) - 1 == listOf(2, 3, 1); listOf(1, 2, 3, 1) - listOf(1) == listOf(2, 3)
that's what's in stdlib
j
Yep
e
the one I wrote is not
j
It would return 
listOf(2,3,1)
e
stdlib has a set optimization which mine doesn't
hence it has different behavior
j
Well, that assumes that 
remainingElementsToRemove.remove(it)
only removes once
And breaks early if it finds the element
e
well, if you convert it to a set there is only one element to remove
if you wanted 
listOf(1, 1, 1) - listOf(1, 1) == listOf(1)
, well mine doesn't do that. you'd need a multiset (or map) to keep track.
Well, that assumes that 
remainingElementsToRemove.remove(it)
only removes once
And breaks early if it finds the element
I don't understand what you mean by that
n
if the stdlib uses a set, doesn't it remove all repetitions that are unrelated to the elements that are requested for removal?
That seems pretty surprising
e
it uses a set on the 
elements
argument, not 
this
n
Ah I see
e
internal implementation detail, it could just not convert and perform the 
in/.contains
query each time, to the same effect (except for runtime)
n
I am open to seeing examples otherwise, but list - list feels really smelly, like anytime you end up using it, there's some earlier issue with your data structures
e
Set - Iterable makes sense, but I tend to agree - Iterable - Iterable is less good. but it's the same implementation either way, so I guess they just generalized it...
j
I think there are a number of inconsistencies. If it's using a set under the hood, then the elements have to map to the same hashcode (and equals? I always forget). If they map to the same hashcode (and equals) then it could be argued that the caller does want to remove all instances of the element because it's the "same" element. So what you deem incorrect may be totally correct for someone else's use case.
e
if your elements have a hashCode() that is inconsistent with equals() they're breaking the Object contract in all sorts of other ways, we can disregard that as a possibility
using hashCode() is meant to be an optimization over using just equals(). it still uses equals(), it just uses hashCode() first
that's how HashSet works
j
If I ask the machine to do 
listOf(1,1,1) - listOf(1)
, 
emptyList()
and 
listOf(1,1)
are both valid answers
So it's not wrong, it's just a very important implementation detail
e
it's documented
j
Excellent, the prosecution defense rests
e
it is somewhat unfortunate that 
.removeAll()
isn't as well documented on the Kotlin side... but it behaves just like Java's 
.removeAll()
so shrug just go read the original Javadoc I guess
n
Set - Iterable is fine. That's just very clearly set substraction; I can't imagine this operation could raise any major question marks about either behavior or implementation here
3 Views
Open in Slack
PreviousNext
Powered by