refactor: improve the Range class

spec/commit_spec.cr

@@ -12,9 +12,10 @@ describe TimeCost::Commit do
       )
 
       commit = TimeCost::Commit.new(
-        commit_hash: "53c01d0db42ac662ed1aff3799d2a92a04e03908",
+        commit_hash: Random::Secure.base64(40),
         datetime: Time.utc,
         author: author,
+        message: "First commit"
       )
       commit.should be_a(TimeCost::Commit)
     end

spec/range_spec.cr

@@ -14,30 +14,35 @@ describe TimeCost::Range do
     email: "jon.snow@example.com"
   )
 	commit_base =  TimeCost::Commit.new(
-    commit_hash: Random::Secure.base64(40),
+    commit_hash: Random::Secure.hex(40),
     author: author,
     datetime: Time.utc,
+    message: "Commit base"
   )
 	commit_overlap_before =  TimeCost::Commit.new(
-    commit_hash: Random::Secure.base64(40),
+    commit_hash: Random::Secure.hex(40),
     author: author,
     datetime: Time.utc - overlap_time,
+    message: "Commit with overlap before"
   )
 	commit_overlap_after =  TimeCost::Commit.new(
-    commit_hash: Random::Secure.base64(40),
+    commit_hash: Random::Secure.hex(40),
     author: author,
     datetime: Time.utc + overlap_time,
+    message: "Commit with overlap after"
   )
 
 	commit_separate_before =  TimeCost::Commit.new(
-    commit_hash: Random::Secure.base64(40),
+    commit_hash: Random::Secure.hex(40),
     author: author,
     datetime: Time.utc - separate_time,
+    message: "Commit separate before"
   )
 	commit_separate_after =  TimeCost::Commit.new(
-    commit_hash: Random::Secure.base64(40),
+    commit_hash: Random::Secure.hex(40),
     author: author,
     datetime: Time.utc + separate_time,
+    message: "Commit separate after"
   )
 
 
@@ -58,6 +63,16 @@ describe TimeCost::Range do
 		end
 	end
 
+  describe ".to_s" do
+		it "must display something" do
+	    range_base = TimeCost::Range.new(
+        commit: commit_base
+      )
+      str = range_base.to_s
+      puts str
+    end
+  end
+
 	describe ".overlap?" do
 		it "must return true when range overlaps range before" do
 	    range_base = TimeCost::Range.new(
@@ -115,5 +130,81 @@ describe TimeCost::Range do
       overlap2.should be_false
 		end
 	end
+
+  describe ".add" do
+    pending "must add the commit to the range"
+
+    pending "must not duplicate existing commits"
+
+    pending "must change the boundaries"
+  end
+
+  describe ".merge" do
+    it "must return a merged range with all commits" do
+	    range_base = TimeCost::Range.new(
+        commit: commit_base
+      )
+      range_before = TimeCost::Range.new(
+        commit: commit_overlap_before
+      )
+      range_after = TimeCost::Range.new(
+        commit: commit_overlap_after
+      )
+
+      range_result1 = range_base.merge(range_before)
+      range_result1.commits.size.should eq(2)
+
+      range_result2 = range_base.merge(range_after)
+      range_result2.commits.size.should eq(2)
+    end
+
+    pending "must not include a commit twice"
+
+    it "must return a merged range with correct boundaries" do
+	    range_base = TimeCost::Range.new(
+        commit: commit_base
+      )
+      range_before = TimeCost::Range.new(
+        commit: commit_overlap_before
+      )
+      range_after = TimeCost::Range.new(
+        commit: commit_overlap_after
+      )
+
+      range_result1 = range_base.merge(range_before)
+      range_result1.time_start.should eq(range_before.time_start)
+      range_result1.time_stop.should eq(range_base.time_stop)
+
+      range_result2 = range_base.merge(range_after)
+      range_result2.time_start.should eq(range_base.time_start)
+      range_result2.time_stop.should eq(range_after.time_stop)
+    end
+
+    it "must fail with error when separate before" do 
+	    range_base = TimeCost::Range.new(
+        commit: commit_base
+      )
+      range_before = TimeCost::Range.new(
+        commit: commit_separate_before
+      )
+
+      expect_raises(TimeCost::Range::MissingOverlapError) do
+        range_base.merge(range_before)
+      end
+    end
+
+    it "must fail with error when separate after" do 
+	    range_base = TimeCost::Range.new(
+        commit: commit_base
+      )
+      range_after = TimeCost::Range.new(
+        commit: commit_separate_after
+      )
+
+      expect_raises(TimeCost::Range::MissingOverlapError) do
+        range_base.merge(range_after)
+      end
+    end
+  end
 end
 

src/timecost/author.cr

@@ -12,5 +12,9 @@ module TimeCost
         (self.email == other_author.email)
       )
     end
+
+    def to_s
+      "#{self.name} <#{self.email}>"
+    end
   end
 end

src/timecost/commit.cr

@@ -4,13 +4,14 @@ require "./author"
 
 module TimeCost
   class Commit
-    property author : Author
-    property commit_hash : String
+    getter author : Author
+    getter commit_hash : String
 
-    property datetime : Time
-    property message : String
+    getter datetime : Time
+    getter message : String
+    getter notes : String
 
-    def initialize(@commit_hash, @datetime=Time, @author=Author, @message="")
+    def initialize(@commit_hash, @datetime, @author, @message, @notes="")
       # @note = nil
     end
   end

src/timecost/git_reader.cr

@@ -40,6 +40,7 @@ module TimeCost
           datetime: Time.parse!(json["date"].to_s, "%F %T %z"),
           author: author,
           message: json["message"].to_s,
+          notes: json["notes"].to_s
         )
       end
 

src/timecost/range.cr

@@ -1,7 +1,10 @@
 require "./commit"
 
 module TimeCost
+
   class Range
+    class MissingOverlapError < RuntimeError ; end
+
     property time_start
     property time_stop
     property commits : Array(Commit)
@@ -24,34 +27,35 @@ module TimeCost
       @commits.first.try &.author 
     end
 
-    def merge(range)
-      #  B -----[----]----
-      #  A  --[----]------
-      #  = ---[------]----
+    def add(commit : Commit) : Range
+    end
 
-      # minimum of both
-      new_start = (
-        if range.time_start < @time_start
-          range.time_start
-        else
-          @time_start
-        end
-      )
+    def merge!(other_range) : Range
+      raise MissingOverlapError.new if @time_stop < other_range.time_start
+      raise MissingOverlapError.new if @time_start > other_range.time_stop
 
-      new_end = (
-        if range.time_stop >= @time_stop
-          range.time_stop
-        else
-          @time_stop
-        end
-      )
+      #  B ------[----]----
+      #  A  --[----]-------
+      #  = ---[-------]----
+
+      # boundaries
+      new_start = [@time_start, other_range.time_start].min
+      new_end = [@time_stop, other_range.time_stop].max
 
       @time_start = new_start
       @time_stop = new_end
-      @commits.concat range.commits
+      @commits.concat other_range.commits
+      self
     end
 
-    def overlap?(range)
+    def merge(other_range) : Range
+      copy = self.dup
+      copy.commits = self.commits.dup
+
+      copy.merge!(other_range)
+    end
+
+    def overlap?(range) : Bool
       result = false
 
       # return early result if ranges come from different authors
@@ -104,27 +108,26 @@ module TimeCost
       return result
     end
 
-    def fixed_start
-      return @time_start + (@epsilon/24.0)
+    def diff_hours : Float
+      return ((@time_stop - @time_start) / (60* 60)).to_f
     end
 
-    def diff : Float
-      return ("%.2f" % ((@time_stop - fixed_start).to_f * 24)).to_f
-    end
-
-    def to_s(show_authors = true)
-      val = "(%s)\t%s - %s\n" % [diff, fixed_start, @time_stop]
-      if show_authors
-        val += "\tby %s\n" % @commits.first.author
+    def to_s(show_authors = true) : String
+      result = String.build do |val|
+        val << "(%.2f)\t%s - %s\n" % [diff_hours, @time_start, @time_stop]
+        if show_authors
+          val << "\tby %s\n" % @commits.first.author.to_s
+        end
+        @commits.each do |commit|
+          lines = commit.message.split(/\n/)
+          r = lines.map_with_index do |line,i| 
+            x = (i == 0) ? "[#{commit.commit_hash[0..7]}]" : " " * 9
+            "\t#{x} #{line}"
+          end.join("\n")
+          val << r + "\n"
+        end
       end
-      @commits.each do |commit|
-        lines = [] of String
-        lines.concat commit.note.split(/\n/)
-        r = lines.map { |s| "\t  %s" % s }.join "\n"
-        r[1] = '*'
-        val += r + "\n"
-      end
-      return val
+      result
     end
   end
 end