Merge pull request #139 from zendesk/craig/edge-calculations

Treat endpoints consistently in calculations
zendesk · Jan 14, 2019 · b10ccfe · b10ccfe
2 parents 0affaca + dcf55bc
commit b10ccfe
Show file tree

Hide file tree

Showing 13 changed files with 117 additions and 77 deletions.
diff --git a/lib/biz/calculation/for_duration.rb b/lib/biz/calculation/for_duration.rb
@@ -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
 
@@ -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
@@ -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)

diff --git a/lib/biz/periods/abstract.rb b/lib/biz/periods/abstract.rb
@@ -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

diff --git a/lib/biz/periods/after.rb b/lib/biz/periods/after.rb
@@ -29,10 +29,6 @@ def weeks
         )
       end
 
-      def relevant?(period)
-        origin < period.end_time
-      end
-
     end
   end
 end
diff --git a/lib/biz/periods/before.rb b/lib/biz/periods/before.rb
@@ -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

diff --git a/lib/biz/time_segment.rb b/lib/biz/time_segment.rb
@@ -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

diff --git a/lib/biz/timeline/abstract.rb b/lib/biz/timeline/abstract.rb
@@ -14,9 +14,9 @@ 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?
           end
       end
 
@@ -26,13 +26,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.new(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
 

diff --git a/lib/biz/timeline/backward.rb b/lib/biz/timeline/backward.rb
@@ -10,8 +10,12 @@ def backward
 
       private
 
-      def occurred?(period, time)
-        period.start_time <= time
+      def occurred?(period, terminus)
+        period.end_time <= terminus
+      end
+
+      def overflow?(*)
+        false
       end
 
       def comparison_period(period, terminus)

diff --git a/lib/biz/timeline/forward.rb b/lib/biz/timeline/forward.rb
@@ -10,8 +10,12 @@ def forward
 
       private
 
-      def occurred?(period, time)
-        period.end_time >= time
+      def occurred?(period, terminus)
+        period.start_time > terminus
+      end
+
+      def overflow?(period, remaining)
+        period.duration == remaining
       end
 
       def comparison_period(period, terminus)

diff --git a/spec/biz_spec.rb b/spec/biz_spec.rb
@@ -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
 

diff --git a/spec/calculation/for_duration_spec.rb b/spec/calculation/for_duration_spec.rb
@@ -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
@@ -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

diff --git a/spec/time_segment_spec.rb b/spec/time_segment_spec.rb
@@ -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
@@ -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
@@ -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
 
@@ -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

diff --git a/spec/timeline/backward_spec.rb b/spec/timeline/backward_spec.rb
@@ -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
@@ -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
-Original file 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