kotlinx.collections.immutable icon indicating copy to clipboard operation
kotlinx.collections.immutable copied to clipboard

Rename removeAll/retainAll to filter/filterNot

Open mcpiroman opened this issue 2 years ago • 2 comments

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.

mcpiroman avatar Jan 02 '23 17:01 mcpiroman

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.

turbochan avatar Mar 19 '24 06:03 turbochan

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.

qurbonzoda avatar Mar 20 '24 10:03 qurbonzoda