[OrderedSet] Add OrderedSet.appending(contentsOf:)

[pm-dev]

Description

This PR adds a new public function on appending(contentsOf:) to OrderedSet. It is functionally equivalent to union(_:) but is more explicit about ordering. I find myself looking for this, rather than union because a mutating append(contentsOf:) already exists.

A counter argument to this addition would be to minimize duplicative APIs as much as possible to reduce API surface area. However, there's already precedent for "alternative" functions, given append(contentsOf:) and formUnion(_:) coexist. And in this case I believe it adds sufficient value to justify the addition.

It's second nature to look for an "append" on ordered collections in Swift. Arrays are ubiquitous and an append(contentsOf:) exists. However, unlike Array, OrderedSet does not support the + operator, so adding this as an alternative to union(_:) seems to fill a gap.

Detailed Design

The implementation is simple, copying the instance to a mutable value then calling append(contentsOf:) and returning the result. This is the same pattern to how union(_:) works.

Documentation

Symbol-level documentation has been added but I don't think it's necessary to update any guides, since this isn't adding any new functionality.

Testing

A new test_appending_Self has been added

Performance

Not profiled.

Source Impact

What is the impact of this change on existing users of this package? Does it deprecate or remove any existing API?

Checklist

• I've read the Contribution Guidelines
• My contributions are licensed under the Swift license.
• I've followed the coding style of the rest of the project.
• I've added tests covering all new code paths my change adds to the project (to the extent possible).
• I've added benchmarks covering new functionality (if appropriate).
• I've verified that my change does not break any existing tests or introduce unexpected benchmark regressions.
• I've updated the documentation (if appropriate).

[xwu]

However, unlike Array, OrderedSet does not support the + operator

Should OrderedSet just support the + operator?

[pm-dev]

Should OrderedSet just support the + operator?

Maybe so. Originally I thought + infers a semantic meaning of "concatenation" which doesn't totally make sense for sets (the Set type doesn't have a + operator). But maybe that's just overthinking it.

[lorentey]

I think this would be an easy addition if appending(contentsOf:) was a standard operation over range replaceable collections. However, it isn't. 😕

OrderedSet is stuck in a weird place between RangeReplaceableCollection and SetAlgebra, not quite able to conform to either protocol, and so it gets the benefits of neither.

As @xwu said, the standard spelling for concatenation over range-replaceable collections is to use the + operator. OrderedSet is not a RangeReplaceableCollection, so the stdlib's extension does not apply. For what it's worth, OrderedSet is closer in spirit to SetAlgebra than RRC, but SetAlgebra does not define + as an alias for union.

I would not have much appetite for introducing the + notation specifically for OrderedSet. While there is some precedent of using + as a stand-in for ∪, it would immediately raise the idea of also introducing ∗ for ⋂, and I don't like the idea of starting on this path. Additionally, I feel it is particularly objectionable that the new + would likely need to be heterogenous, accepting an arbitrary sequence as its right operand. So far this package has been careful to avoid defining any operators, much less generic ones. I do not wish to step through that door unless it is the only way to solve a vexing issue (which this really isn't).

Adding appending(contentsOf:) is far less scary an idea. I think append(contentsOf:)/formUnion is a useful analogue.
Please let me think about it for a short time! I've provisionally put this on the 1.2 milestone, to encourage a decision.

[lorentey]

OK, let's do this!

[lorentey]

@swift-ci test

[lorentey]

I'll cherry pick this to the release/1.2 branch, in a separate (bulk) PR