Skip to content

Commit

Permalink
Treat endpoints consistently in calculations
Browse files Browse the repository at this point in the history
Before, a lack of handling of time segment endpoints in calculations
consistent with how they are handled elsewhere in the gem (that is,
*including* the start time and *excluding* the end time) was causing
weird behavior and inconsistencies across both `#for` and `#until`
period generation methods, which filtered down to high-level
calculations.

When looking forward in time, if consumes, or lands at the end of
a period, one extra "moment" period should be generated to compensate
for the fact that the end time of a time segment is not considered part
of that segment. Under similar circumstances when looking backward in
time, since the start time of a time segment is considered inclusive to
that time segment, calculations that end on or at that moment should not
proceed to generate the next moment period.

A nice side effect of making this behavior consistent at the lowest
level of the engine is that we can remove special-case handling that was
added to the high-level calculation object in support of zero-scalar
calculations. We can simply use the period and timeline objects normally
and get the correct behavior. Woohoo!

Running the benchmark suite, we don't see a meaningful delta in
performance characteristics with these changes.

Before:
```
Biz.configure do |config|
  config.hours = {
    mon: {'00:00' => '24:00'},
    tue: {'00:00' => '24:00'},
    wed: {'00:00' => '24:00'},
    thu: {'00:00' => '24:00'},
    fri: {'00:00' => '24:00'}
  }
end

monday = Time.utc(2019, 1, 14)

Biz.time(1, :days).before(monday) #=> 2019-01-11 00:00:00 UTC
Biz.time(2, :days).before(monday) #=> 2019-01-10 00:00:00 UTC
Biz.time(4, :days).before(monday) #=> 2019-01-08 00:00:00 UTC
Biz.time(5, :days).before(monday) #=> 2019-01-05 00:00:00 UTC <- Expected 1/7/19
Biz.time(0, :days).before(monday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19
Biz.time(0, :days).before(monday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct

tuesday = Time.utc(2019, 1, 15)

Biz.time(0, :days).before(tuesday) #=> 2019-01-15 00:00:00 UTC
Biz.time(1, :days).before(tuesday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19
Biz.time(1, :day).before(tuesday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct
Biz.time(5, :days).before(tuesday) #=> 2019-01-08 00:00:00 UTC

friday = Time.utc(2019, 1, 11)
Biz.time(24, :hours).after(friday) #=> 2019-01-12 00:00:00 UTC <- 1/14/19 expected
Biz.time(1, :day).after(friday) #=> 2019-01-14 00:00:00 UTC <- Correct
```

After:
```
>> Biz.configure do |config|
?>       config.hours = {
?>           mon: {'00:00' => '24:00'},
?>           tue: {'00:00' => '24:00'},
?>           wed: {'00:00' => '24:00'},
?>           thu: {'00:00' => '24:00'},
?>           fri: {'00:00' => '24:00'}
>>       }
>>   end
>>
>> monday = Time.utc(2019, 1, 14)
=> 2019-01-14 00:00:00 UTC
>>
>> Biz.time(1, :days).before(monday) #=> 2019-01-11 00:00:00 UTC
=> 2019-01-11 00:00:00 UTC
>> Biz.time(2, :days).before(monday) #=> 2019-01-10 00:00:00 UTC
=> 2019-01-10 00:00:00 UTC
>> Biz.time(4, :days).before(monday) #=> 2019-01-08 00:00:00 UTC
=> 2019-01-08 00:00:00 UTC
>> Biz.time(5, :days).before(monday) #=> 2019-01-05 00:00:00 UTC <- Expected 1/7/19
=> 2019-01-07 00:00:00 UTC
>> Biz.time(0, :days).before(monday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19
=> 2019-01-14 00:00:00 UTC
>> Biz.time(0, :days).before(monday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct
=> 2019-01-14 00:00:01 UTC
>>
>> tuesday = Time.utc(2019, 1, 15)
=> 2019-01-15 00:00:00 UTC
>>
>> Biz.time(0, :days).before(tuesday) #=> 2019-01-15 00:00:00 UTC
=> 2019-01-15 00:00:00 UTC
>> Biz.time(1, :days).before(tuesday) #=> 2019-01-12 00:00:00 UTC <- Expected 1/14/19
=> 2019-01-14 00:00:00 UTC
>> Biz.time(1, :day).before(tuesday + 1) #=> 2019-01-14 00:00:01 UTC <- Correct
=> 2019-01-14 00:00:01 UTC
>> Biz.time(5, :days).before(tuesday) #=> 2019-01-08 00:00:00 UTC
=> 2019-01-08 00:00:00 UTC
>> friday = Time.utc(2019, 1, 11)
=> 2019-01-11 00:00:00 UTC
>> Biz.time(24, :hours).after(friday) #=> 2019-01-12 00:00:00 UTC <- 1/14/19 expected
=> 2019-01-14 00:00:00 UTC
>> Biz.time(1, :day).after(friday) #=> 2019-01-14 00:00:00 UTC <- Correct
=> 2019-01-14 00:00:00 UTC
```

Fixes #138.
  • Loading branch information
craiglittle committed Jan 13, 2019
1 parent 0affaca commit 970ac46
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 74 deletions.
39 changes: 15 additions & 24 deletions lib/biz/calculation/for_duration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,10 @@ def self.unit
def self.time_class
Class.new(self) do
def before(time)
return moment_before(time) if scalar.zero?

advanced_periods(:before, time).last.start_time
end

def after(time)
return moment_after(time) if scalar.zero?

advanced_periods(:after, time).last.end_time
end

Expand All @@ -52,31 +48,34 @@ def advanced_periods(direction, time)
def self.day_class
Class.new(self) do
def before(time)
return moment_before(time) if scalar.zero?

periods(:before, time).first.end_time
end

def after(time)
return moment_after(time) if scalar.zero?

periods(:after, time).first.start_time
end

private

def periods(direction, time)
schedule.periods.public_send(
direction,
advanced_time(direction, schedule.in_zone.local(time))
)
schedule
.periods
.public_send(
direction,
advanced_time(direction, schedule.in_zone.local(time))
)
end

def advanced_time(direction, time)
schedule.in_zone.on_date(
schedule.dates.days(scalar).public_send(direction, time),
DayTime.from_time(time)
)
schedule
.in_zone
.on_date(
schedule
.dates
.days(scalar)
.public_send(direction, time.to_date),
DayTime.from_time(time)
)
end
end
end
Expand All @@ -100,14 +99,6 @@ def unit
self.class.unit
end

def moment_before(time)
schedule.periods.before(time).first.end_time
end

def moment_after(time)
schedule.periods.after(time).first.start_time
end

[
*%i[second seconds minute minutes hour hours].map { |unit|
const_set(unit.to_s.capitalize, time_class)
Expand Down
5 changes: 2 additions & 3 deletions lib/biz/periods/abstract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ def timeline
def periods
Linear.new(week_periods, shifts, selector)
.lazy
.select { |period| relevant?(period) }
.map { |period| period & boundary }
.map { |period| period & boundary }
.reject(&:disjoint?)
.flat_map { |period| active_periods(period) }
.reject { |period| on_holiday?(period) }
.reject(&:empty?)
end

def week_periods
Expand Down
4 changes: 0 additions & 4 deletions lib/biz/periods/after.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ def weeks
)
end

def relevant?(period)
origin < period.end_time
end

end
end
end
4 changes: 0 additions & 4 deletions lib/biz/periods/before.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ def weeks
.downto(Week.since_epoch(Time.big_bang))
end

def relevant?(period)
origin > period.start_time
end

def active_periods(*)
super.reverse
end
Expand Down
18 changes: 11 additions & 7 deletions lib/biz/time_segment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,33 +24,37 @@ def initialize(start_time, end_time)
:date

def duration
Duration.new(end_time - start_time)
Duration.new([end_time - start_time, 0].max)
end

def endpoints
[start_time, end_time]
end

def empty?
start_time >= end_time
start_time == end_time
end

def disjoint?
start_time > end_time
end

def contains?(time)
(start_time...end_time).cover?(time)
end

def &(other)
lower_bound = [self, other].map(&:start_time).max
upper_bound = [self, other].map(&:end_time).min

self.class.new(lower_bound, [lower_bound, upper_bound].max)
self.class.new(
[self, other].map(&:start_time).max,
[self, other].map(&:end_time).min
)
end

def /(other)
[
self.class.new(start_time, other.start_time),
self.class.new(other.end_time, end_time)
].reject(&:empty?).map { |potential| self & potential }
].reject(&:disjoint?).map { |potential| self & potential }
end

private
Expand Down
21 changes: 15 additions & 6 deletions lib/biz/timeline/abstract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ def until(terminus)
periods
.map { |period| period & comparison_period(period, terminus) }
.each do |period|
yield period unless period.empty?

break if occurred?(period, terminus)

yield period unless period.disjoint?

break if elapsed?(period, terminus)
end
end

Expand All @@ -26,13 +28,20 @@ def for(duration)
remaining = duration

periods
.map { |period| period & duration_period(period, remaining) }
.each do |period|
yield period unless period.empty?
if overflow?(period, remaining)
remaining = Duration.seconds(0)

yield period
else
scoped_period = period & duration_period(period, remaining)

remaining -= scoped_period.duration

remaining -= period.duration
yield scoped_period unless scoped_period.disjoint?

break unless remaining.positive?
break unless remaining.positive?
end
end
end

Expand Down
8 changes: 8 additions & 0 deletions lib/biz/timeline/backward.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,17 @@ def backward
private

def occurred?(period, time)
period.end_time <= time
end

def elapsed?(period, time)
period.start_time <= time
end

def overflow?(*)
false
end

def comparison_period(period, terminus)
TimeSegment.new(terminus, period.end_time)
end
Expand Down
10 changes: 9 additions & 1 deletion lib/biz/timeline/forward.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,15 @@ def forward
private

def occurred?(period, time)
period.end_time >= time
period.start_time > time
end

def elapsed?(period, time)
period.end_time > time
end

def overflow?(period, duration)
period.duration == duration
end

def comparison_period(period, terminus)
Expand Down
2 changes: 1 addition & 1 deletion spec/biz_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
it 'delegates to the top-level schedule' do
expect(
described_class.time(2, :hours).after(Time.utc(2006, 1, 1))
).to eq Time.utc(2006, 1, 8, 12)
).to eq Time.utc(2006, 1, 15, 11)
end
end

Expand Down
8 changes: 4 additions & 4 deletions spec/calculation/for_duration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,10 @@
end

describe '#after' do
let(:time) { Time.utc(2006, 1, 4, 15, 30) }
let(:time) { Time.utc(2006, 1, 4, 14, 30) }

it 'returns the forward time after the elapsed duration' do
expect(calculation.after(time)).to eq Time.utc(2006, 1, 4, 17)
expect(calculation.after(time)).to eq Time.utc(2006, 1, 4, 16)
end

context 'when the scalar is zero' do
Expand Down Expand Up @@ -181,10 +181,10 @@
end

describe '#after' do
let(:time) { Time.utc(2006, 1, 4, 14) }
let(:time) { Time.utc(2006, 1, 4, 13) }

it 'returns the forward time after the elapsed duration' do
expect(calculation.after(time)).to eq Time.utc(2006, 1, 4, 17)
expect(calculation.after(time)).to eq Time.utc(2006, 1, 4, 16)
end

context 'when the scalar is zero' do
Expand Down
48 changes: 39 additions & 9 deletions spec/time_segment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@
Biz::Duration.new(end_time - start_time)
)
end

context 'when the time segment is disjoint' do
let(:time_segment) { described_class.new(end_time, start_time) }

it 'returns a zero duration' do
expect(time_segment.duration).to eq Biz::Duration.new(0)
end
end
end

describe '#start_time' do
Expand Down Expand Up @@ -74,11 +82,37 @@
end
end

context 'when the start time is after the end time' do
let(:time_segment) { described_class.new(end_time + 1, end_time) }

it 'returns false' do
expect(time_segment.empty?).to eq false
end
end
end

describe '#disjoint?' do
context 'when the start time is before the end time' do
let(:time_segment) { described_class.new(start_time, start_time + 1) }

it 'returns false' do
expect(time_segment.disjoint?).to eq false
end
end

context 'when the start time is equal to the end time' do
let(:time_segment) { described_class.new(start_time, start_time) }

it 'returns false' do
expect(time_segment.disjoint?).to eq false
end
end

context 'when the start time is after the end time' do
let(:time_segment) { described_class.new(end_time + 1, end_time) }

it 'returns true' do
expect(time_segment.empty?).to eq true
expect(time_segment.disjoint?).to eq true
end
end
end
Expand Down Expand Up @@ -132,10 +166,8 @@
let(:other_start_time) { Time.utc(2006, 1, 1) }
let(:other_end_time) { Time.utc(2006, 1, 2) }

it 'returns a zero-duration time segment' do
expect(time_segment & other).to eq(
described_class.new(start_time, start_time)
)
it 'returns a disjoint time segment' do
expect(time_segment & other).to be_disjoint
end
end

Expand Down Expand Up @@ -191,10 +223,8 @@
let(:other_start_time) { Time.utc(2006, 2, 1) }
let(:other_end_time) { Time.utc(2006, 2, 7) }

it 'returns a zero-duration time segment' do
expect(time_segment & other).to eq(
described_class.new(other_start_time, other_start_time)
)
it 'returns a disjoint time segment' do
expect(time_segment & other).to be_disjoint
end
end
end
Expand Down
8 changes: 5 additions & 3 deletions spec/timeline/backward_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
end
end

context 'when the terminus at the end of the first period' do
context 'when the terminus is at the end of the first period' do
let(:terminus) { Time.utc(2006, 2) }

it 'returns no periods' do
Expand Down Expand Up @@ -106,8 +106,10 @@
context 'when the duration is zero' do
let(:duration) { Biz::Duration.new(0) }

it 'returns no periods' do
expect(timeline.for(duration).to_a).to eq []
it 'returns the first active moment backward in time' do
expect(timeline.for(duration).to_a).to eq [
Biz::TimeSegment.new(Time.utc(2006, 2), Time.utc(2006, 2))
]
end
end

Expand Down

0 comments on commit 970ac46

Please sign in to comment.