Can the compact() shorthand for compactMap { $0 } be implemented now?

The review for SE-0187 says:

Therefore, SE-0187 is accepted, with the revision that the new name be Sequence.compactMap(_:), and with the agreement that we will add Sequence.compact() when it is possible to do so.

Since filtering out nil values from a collection is such a common operation, I think compact() would be a nice improvement – can it now be implemented? (If not, what features are we waiting for?) This works for me, but I don’t know if it’s the right thing:

extension Sequence {
    func compact<T>() -> [T] where Element == Optional<T> {
        return compactMap { $0 }
    }
}
10 Likes

It's probably the most concise implementation - but as compactMap is conceptionally the combination of compact and map, I'd prefer something more verbose...

extension Sequence {
    func compact<T>() -> [T] where Element == Optional<T> {
        return reduce([T]()) {
            if let value = $1 {
                var result = $0
                result.append(value)
                return result
            } else {
                return $0
            }
        }
    }

    // or, the old-fashioned way which I'd prefer...
    func compact<T>() -> [T] where Element == Optional<T> {
        var result = [T]()
        for element in self {
            guard let element = element else { continue }
            result.append(element)
        }
        return result
    }
}

Based on the proposal text and my recollection, I think the feature we are waiting for is the ability to write [edit thanks to @DevAndArtist]:
extension<T> Sequence where Element == Optional<T> { ... }

1 Like

That’s what I thought, but then I discovered the workaround with a generic method. Is there a reason not to go with that in the meantime?

Because it cannot later be removed after ABI stability. As you demonstrate, you can use it yourself with minimal fuss if you wish.

Sure, but the same could be said about many of the syntactic improvements in the standard library. I didn’t realize the problem with ABI stability though, that’s a pity. Thank you!

Why would we want to remove it later? The proposed implementation works just fine. I don’t see why we’d replace it even once we have the features needed to write the other implementation.

1 Like

The idea at the time of the proposal was that this method should only be added for sequences of optional elements.

Unless I’m mistaken, the implementation outlined here was possible even at the time of the proposal, and the decision was not to add it but to wait for the desired design.

Additionally, it may not be possible to have both this and the desired future design both in the standard library without causing ambiguity at the point of use that raises a compiler error. Nor does this method provide functionality above the desired future design—they would be strictly redundant. Which is to say that an addition now (which was not approved) would likely preclude the desired addition later (which was approved).

The member is uncallable unless the Element is optional, right? So is the worry that it’ll show up in code completion or something?

And bloating witness tables, etc., I guess? I’m not certain, but I do know that this implementation was not approved and strongly suspect that it would preclude the design that was approved.

I don’t think it would appear in witness tables—it’s not a customization point, so it would be statically called.

I think we’ll have to see if someone can raise any firsthand objections to it. For my part, I don’t think I knew that this implementation was possible—constraining the type parameter in this way didn’t occur to me.

extension Sequence where Element : Optional { ... }

Any chance that you meant this, or omitting it should also imply the same code?

extension<T> Sequence where Element == Optional<T> { ... }

Was the colon a typo in your snippet?

1 Like

:) Yes, I actually meant that. Thanks.

2 Likes

Me too. I had believed that it was impossible to implement compact() without parameterized extensions.

@xwu
Because ABI stability has not done, if there are no other problems, is it no good to have the proposed implementation of compact() for now and replace it when parameterized extensions get available before ABI stability?

1 Like

Will parametrized extensions be available before Swift 5? I realize Swift 5 was delayed a bit, but I didn't think that anyone was even working on implementing parameterized extensions yet.

Parameterized extensions are not planned for Swift 5 AFAIK, but previous comments by the core team have suggested they're happy to wait for them to implement compact() rather than hacking together a subpar implementation (we've always known we could implement it if we added a silly _OptionalProtocol protocol to the standard library). Personally, I don't think this implementation of compact() is a hack, so I think we should go for it.

In that case, should we also add the flatten (or should it be compact?) to Optional?

extension Optional {
    public func flatten<T>() -> T? where Wrapped == T? {
        return flatMap { $0 }
    }
}
1 Like

Well, it isn't not a hack, in that compact() isn't meant to be a generic function. The accepted proposal was to wait for parameterized extensions. To revisit that decision, we'd need another proposal. :man_shrugging:

If the objection is purely that no one thought of the proposed implementation before, then I think a full on proposal is overkill.

It seems that the implementation was considered previously: