Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for interval offsets #28

Merged
merged 10 commits into from Dec 28, 2018
Merged

Add support for interval offsets #28

merged 10 commits into from Dec 28, 2018

Conversation

WilsonGramer
Copy link
Contributor

Adds the ability to offset a Task by a specific interval from its Plan, such so that the Task calculates its next run based on adding said interval to the Plan's next interval. This can be useful if one has a preexisting Plan and needs to uniformly adjust the times at which its Task runs (eg. timezone differences, etc.).

Usage examples:

plan.do(offsetBy: 3.hours) { ... }
let calculateOffset = { () -> Interval? in
  // some computation to calculate offsets on the fly
}
plan.do(offsetBy: calculateOffset) { ... }

Adds the ability to offset a Task by a specific interval from its Plan, such so that the Task calculates its next run based on adding said interval to the Plan's next interval.
@codecov-io
Copy link

codecov-io commented Dec 5, 2018

Codecov Report

Merging #28 into master will increase coverage by 0.09%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
+ Coverage   90.86%   90.96%   +0.09%     
==========================================
  Files          16       16              
  Lines         832      841       +9     
==========================================
+ Hits          756      765       +9     
  Misses         76       76

Although negative interval offsets are allowed, they shouldn't make the Plan have a negative interval.
This was preventing functions from being passed to the offsetBy: parameter without issue, so it was removed.
@luoxiu
Copy link
Owner

luoxiu commented Dec 10, 2018

Hi @Wilsonator5000 , Sorry for my late reply! I like the offset idea! But i think it should be part of the plan, not the task, so this may be better:

extension Plan {
    // just a draft
    func offset(_ body: @autoclosure @escaping () -> Interval?) -> Plan {
        return Plan.make { () -> AnyIterator<Interval> in
            let it = self.makeIterator()
            return AnyIterator {
                if let next = it.next() {
                    return next + (body() ?? 0.second)
                }
                return nil
            }
        }
    }
}

And since it's actually a transformation, a map operator seems better, what do you think?

@WilsonGramer
Copy link
Contributor Author

Hi @jianstm, your method makes sense too! A map wouldn't work because it would convert the iterator to an array, and we want the the intervalOffset body to be evaluated when the next run time is calculated (which could be far in the future). I'll look into the iterator method, though.

@luoxiu luoxiu merged commit cc52ab6 into luoxiu:master Dec 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants