Rename subtractAll to removeAll

[allanrenucci]

In the 2.13 collection library, if you want to delete multiple elements from a mutable collection, the operation is called subtractAll. In java.util.Collections, it is called removeAll. I propose to rename subtractAll to removeAll.

The name removeAll is currently used in immutable.Map to define an operation that creates a new collection by removing all elements of another. To me it looks very similar to the diff operation. Can we reuse this name? Or use subtractAll maybe?

[julienrf]

FTR we also have a removeAll operation in mutable.ArrayDeque. If we rename subtractAll to removeAll we will have to find another name for ArrayDeque#removeAll (I’m not sure how useful is this operation, though…).

In the case of immutable.Map, we have the -- operator as an alias for removeAll and - as an alias for remove. If we decide to not use removeAll here, we should consistently rename remove as well. We could use subtract/subtractAll. In immutable.Set this operation is named excl. Maybe we should use the same name for both immutable.Map and immutable.Set? Should we use past tenses like we do in immutable.Seq (appended, prepended)?

[szeiger]

remove operations take a key / index, subtract operations take a value. These appear to be used consistently in Seq and Map types.

In Set key and value are the same. Our current naming convention favors the key interpretation. diff makes sense for sets but not really for sequences and maps.

So I don't think we should try to unify this any further or change the names. But I would consider past tense names. Or go the other way around and abandon the new past tense names entirely? They are not used consistently and with standard names such as map being dicatated by language syntax there is no way to use them consistently.

[szeiger]

I looked through the immutable collections for other naming inconsistencies:

• (IntMap, LongMap).modifyOrRemove - Should it be transformedOrRemoved?
• (IntMap, LongMap).transform - transformed?
• Set.excl - Consistent with incl, could also be removed
• Map.remove - removed?
• (Set, Map).removeAll - removedAll?

With the inconsistent use of past tense in general there's no strong case for any of them. At least there are no inconsistencies in related methods in immutable collections (e.g. we have some remove methods but no removed).

I'll reschedule this for RC1 to take another look but for now I'll keep the bikeshed closed and not open a PR to rename anything.

[lrytz]

I think we talked about this at some point but cannot find the past discussion. IRC, the conclusion was that in-place operations should be named xInPlace, copying operations should have the direct name (map, transform, remove). The special case is update (instead of updateInPlace), because of assignmnent syntax.

[szeiger]

This means we'd have to rename all the mutating method, so on mutable.Map you would have removeInPlace instead of remove. That doesn't feel right.

I think we have to make a distinction between methods that transform a collection and those that modify an existing collection. The difference is that a transformation runs all elements of the collection through some function which determines the result. This includes methods such as map, flatMap, filter, collect, transform. The canonical versions of these methods (with the "natural" name) return a new collection. They are available on mutable and immutable collections alike, often defined in a supertrait in the collection package. Any mutable in-place version must have an explicit InPlace suffix.

On the other hand we have methods that modify an existing collection, usually by adding or removing a single element or a number of elements, or by updating individual elements by index or key. Except for basic concatenation they are not shared between mutable and immutable collections. The "natural" names feel right for mutable collections (like mutable.Map.remove). They also feel right for immutable collections but to avoid ambiguities we can use a past tense form for those.

Based on these rules immutable.*Map.modifyOrRemove and immutable.*Map.transform are fine as they are. immutable.Map.remove and .removeAll should be renamed to removed / removedAll (see https://github.com/scala/scala/pull/7494).

[szeiger]

Removing blocker label and adding docs. All the renames that should be done have been merged but the naming rules should find their way into the official docs.

[GKuldeepak-Knoldus]

Hi Everyone, Can I pick this issue ?

[SethTisue]

This issue, like other issues on the 3.x milestone, isn't currently eligible to progress, because we can't break bidirectional binary compatibility of the Scala standard library in Scala 2 ever again, and even in Scala 3 the timing of next bincompat break isn't determined yet. see https://github.com/scala/scala-dev/issues/661 for background