Add `filter(_:orElse:)` which takes a parameter for the failure error

[tikitu]

The type of the filter BrightFutures already has is

Future<V, E> -> (V -> Bool) -> Future<V, BrightFuturesError<E>>

That's annoying if you want to use filter in a chain of operations that share the same error type: a.flatMap(b).flatMap(c).filter(p).flatMap(d) where all the error types of a,b,c match. Suddenly d has to fail with a BrightFuturesError<E> instead of the E the others do: and if we have another filter somewhere in the chain we get BrightFuturesError<BrightFuturesError<E>>... A prettier solution at the type level is to convert filter-failure into an error, but one that matches the error type of the chain.

(Side note: this will get more compelling when generic typealiases land: typealias NetworkOp<A, B> = A -> Future<B, NetworkErrorEnum>.)

One can sort of work around this with mapError but it's rather ugly:

a.filter(p).mapError { error in
    switch error {
    case .NoSuchElement:
        return .MyOwnErrorCase
    case .External(let originalError):
        return originalError
    default: // trust the BrightFutures docs that this won't happen (ouch)
        fatalError("Oops oh well")
    }
}

This filter version instead lets you specify the error to return when the filter predicate fails. I called the argument orElse: for no very compelling reason.

[Thomvis]

Hi @tikitu, thanks for the PR!

I think you're right in that it is not very nice to use filter combined with your custom error types. Your solution definitely is an improvement, but I'm thinking of an alternative that I'd like to get your opinion on:

We could define a protocol BrightFuturesErrorType that defines a method called noSuchElementError(). We could then define (a variant of) filter on Futures that have an error type conforming to BrightFuturesErrorType. The implementation of filter would then create an instance of the custom error type. The only thing you'd have to do is make your error type conform to the protocol and everything would work nicely.

Your proposed solution is a little bit more flexible (because you can return any error), but also adds a second parameter.

What do you think?

[tikitu]

For the use case that prompted this PR I definitely want the second parameter: it means I can use two different filter calls in a futures chain and have each return a different error enum case. Using a protocol pushes the decision "things I might want to filter on" into the (extension on the) error type, which means it has to have one (context-independent) answer.

My use case is something like this (not showing you the real thing because it's full of my custom operators...):

authenticateUser(email: email, password: password)
.filter(allSitesResponded, orElse: .SitesDownForMaintenance)
.filter(userHasAccessToAtLeastOneSite, orElse: .UsernameOrPasswordNotRecognised)
.flatMap(getUserChoiceOfWhichSiteToEnter)
.flatMap(getHomepage)
.filter(homepageHasFeatureX, orElse: .FeatureXNotSupported)

[MaxDesiatov]

+1 on this, just stumbled upon a similar use case and filter(_:orElse:) would perfectly fit my needs

[Thomvis]

Sorry for the inactivity. I've revisited your proposal and unfortunately am going to close this PR because I believe it doesn't add enough value for it to be part of the library. I also believe it stretches the meaning of filtering.

I would implement this same pattern using flatMap. It could look something like this:

authenticateUser(email: email, password: password).flatMap { val in
  if !allSitesResponded(val) {
    return Future(error: .SitesDownForMaintenance)
  }
  return Future(value: val)
}

I agree this doesn't look as nice as the one-liner you are using, but for that you could create your own extension on Future that lives in your project that calls flatMap.

Thanks for your time, I hope you understand!