Thomvis / BrightFutures

Write great asynchronous code in Swift using futures and promises
MIT License
1.9k stars 184 forks source link

@discardableResult for andThen? #160

Closed zgthompson closed 7 years ago

zgthompson commented 7 years ago

I believe andThen should have a discardable result to remain consistent with onSuccess and onFailure.

Thomvis commented 7 years ago

Thanks for your suggestion.

andThen returns a new future that can be used to control the order in which callbacks are executed. If you don't care about the order, use onSuccess etc. If you do care about the order, you'll be using andThen and do something with the result.

Perhaps I am missing a use case here, can you provide some more details?

tdesert commented 7 years ago

We sometimes use andThen to prevent code redundancy when some logic should be executed in both onSuccess and onFailure callbacks. As an example, you may want to update the UI of your view controller on both success and failure:

self.startActivityIndicator()
service.performAPICall.onSuccess {
    doSomeWorkOnSuccess()
}.onFailure {
    showErrorMessage()
}.andThen {
    self.stopActivityIndicator()
}

In this case the result of andThen will never be used.

Thomvis commented 7 years ago

Did you consider using onComplete instead of andThen?

tdesert commented 7 years ago

No, I guess I missed this one. :) Many thanks !

zgthompson commented 7 years ago

I am using andThen in a similar way as @tdesert. I hadn't considered using onComplete. I think semantically andThen reads better for someone who may not be familiar with futures, since onSuccess/onFailure/andThen conceptually matches try/catch/finally or try/catch/defer. I'd be interested to hear others opinions though, since that is just my interpretation.

Another use case I have is in my unit tests. I often have an onSuccess block followed by an onFailure block followed by an andThen block. The andThen block is where my XCTestExpectation gets fulfilled. If I change that to an onComplete block is it guaranteed to run after the onSuccess and onFailure?

Thomvis commented 7 years ago

If I change that to an onComplete block is it guaranteed to run after the onSuccess and onFailure?

No, it's not. But the andThen block is also not guaranteed to run after onSuccess and onFailure. The only thing that is guaranteed is that completion handlers added to the result of andThen are executed after the closure given to andThen.

That's also why it is not the same as defer.

You could consider using onComplete instead of onSuccess and onFailure, and switch on the result inside the closure and do the final tasks (e.g. fulfilling expectations) after the switch.

zgthompson commented 7 years ago

No, it's not. But the andThen block is also not guaranteed to run after onSuccess and onFailure.

Ah, I was afraid of that. Interestingly enough, it hasn't appeared to cause an issue with my tests. I like how the code looks using the blocks as opposed to a switch statement, but to guarantee the effect I'm looking for I will make your suggested changes. Thanks for your help.