Skip to content
This repository has been archived by the owner on Aug 6, 2024. It is now read-only.

Adds OneShotTimer and OneShotDispatch Timer #10

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JeanAzzopardi
Copy link

It might be a good idea to have a OneShotTimer, i.e. a timer that can only be run once after a certain delay.

I've done some refactoring to RepeatingTimer in order to change the type of the closure. , so now there are 2 closure types:

/**
The closure to execute when the timer fires.

- parameter timer:   The timer that fired.
- parameter count:   The current invocation count. The first count is 0.
*/
public typealias RepeatedExecutionClosure = ((RepeatingTimer, Int) -> Void)

/**
 The closure to execute when the timer fires.

 - parameter timer:   The timer that fired. 
 */
public typealias OneShotExecutionClosure = ((OneShotTimer) -> Void)

I realise that this is a non-trivial change, but I'd welcome any feedback.

Refactors ExecutionClosures to RepeatedExecutionClosure and OneShotExecutionClosure
@comyar
Copy link
Owner

comyar commented Aug 9, 2016

Very cool, thanks for the contribution. I wonder how much utility this would provide over dispatch_after. It could be useful for games though, especially when wanting to handle scheduled events and pausing. What were your thoughts on use cases?

Anyway, I'll be traveling through next week so I won't be able to seriously review this for a while, but overall I like the idea of including this.

@JeanAzzopardi
Copy link
Author

Thanks for your comment, I'm working on an app with multiple timers, some repeating, some one-shot (scheduled events as you said). I liked the API of Chronos, so I thought it would be great to keep the same API for one-shot timers.


- parameter now: true, if the timer should fire immediately.
*/
public func start(now: Bool) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extending the Timer protocol becomes sort of awkward because of the start function here, right? For a OneShotTimer, the now argument doesn't really make much sense.

I think what we should do is change the declaration of the start function in Timer from func start(now: Bool) to func start(). Then we add func start(now: Bool) to the RepeatingTimer protocol. What do you think?

@comyar
Copy link
Owner

comyar commented Aug 25, 2016

Sorry for the late response to this, overall it looks good. Only some minor comments throughout, really like that you also added tests and maintained the code style 😄.

I'd like to see the changes to the Timer protocol made before pulling this into master; I can make those changes if you like, otherwise you can include them in this pull request.

Regarding naming, I'm indifferent either way we go on it, I was just interested in getting your thoughts on it. Thanks again for the contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants