Thomvis / BrightFutures

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

Add earliest dispatch time functionality to AsyncType #198

Closed mathebox closed 3 years ago

mathebox commented 6 years ago

This pull request adds the functionality to AsyncType to be dispatched earliest at the given deadline. This is similar to delay(). However instead of always waiting for a fixed interval, earliest(at:) will dispatch immediately if the deadline has already passed. In other words, earliest(at:) ensures a minimum time before the future completes.

Motivation When using BrightFutures in combination with network requests, you can't know when the server will respond to your request. If you have some animations for such network requests (e.g. activity indicators), you want the user interface to behave in a smooth manner. This includes no rapid UI changes. With earliest(at:), you can ensure that the user will at least catch a glimpse at the activity indicator when having a fast internet connection. On the other hand, there is no need for waiting an additonal interval when the network request returns after two seconds.

Example

spinner.startAnimation()
doNetworkTask().earliest(at: 500.milliseconds.fromNow).onComplete { _ in
    spinner.stopAnimation()
}
Thomvis commented 6 years ago

Thanks for your contribution!

I think the use case you're describing is valid and should be supported by BrightFutures. I am however not sure if we need to add specific API for this use case, since it is also possible to achieve it using existing functionality. I think the following is equivalent to what you're trying to achieve:

spinner.startAnimation()
doNetworkTask().zip(Future<Void, NoError>(value: ()).delay(.seconds(1))).onComplete { _ in
    spinner.stopAnimation()
}

It's definitely not as nice as your version, but that is in part due to the clumsy way completed futures are created. Perhaps that should be improved upon.

What do you think?

mathebox commented 6 years ago

Haha.. good catch! I haven't thought about solving it in this way.

But I still prefer my proposed implementation since it conveys the intention more clearly and the type of the future's result isn't changed to a tuple (Value, Void).

mathebox commented 5 years ago

I rebased the branch to incorporate the latest changes on master. @Thomvis Should I also add a section for earliest(at:) to the readme?

Thomvis commented 3 years ago

I'm closing this because it went nowhere, due to my own inactivity. If somebody still feels this is needed, feel free to open an issue.