search

Kotlin / kotlinx.collections.immutable

Immutable persistent collections for Kotlin
Apache License 2.0
1.12k stars 56 forks source link

Rename removeAll/retainAll to filter/filterNot #139

Open mcpiroman opened 1 year ago

mcpiroman commented 1 year ago

Current naming is confusing In stdlib removeAll/retainAll modify collections in place, while filter/filterNot return new ones. In kotlinx.collections.immutable removeAll/retainAll return ones and filter/filterNot are taken from stdlib, so they do the same.

This also relates to #123, where persistentList.filter { it % 2 == 0 }.toPersistentList() could be written as persistentList.retainAll { it % 2 == 0 } but the letter is less intuitive when one is used to stdlib.

turbochan commented 3 months ago

I think one issue here is that filter/filterNot() in the stdlib are extension functions, while the removeAll/retainAll() in the immutable library are member functions that I think are meant to have the same names as the stdlib's MutableCollection functions?

Although I'm not sure where PersistentCollection.removeAll(predicate: (E) -> Boolean) fits into all this, as that is not something from MutableCollection, but instead seems more like the stdlib's MutableIterable<T>.removeAll(predicate: (T) -> Boolean) extension function.

qurbonzoda commented 3 months ago

Thank for your feedback! A similar issue is discussed here: https://github.com/Kotlin/kotlinx.collections.immutable/issues/8

We are aware of the possible confusion, but we have not yet found a satisfactory naming scheme. In the future, we will definitely seek to address this issue.