Refactor TimeSeries artifacts

app/helpers/application_helper.rb

@@ -53,7 +53,7 @@ module ApplicationHelper
 
   def trend_styles(trend)
     fallback = { bg_class: "bg-gray-500/5", text_class: "text-gray-500", symbol: "", icon: "minus" }
-    return fallback if trend.nil? || trend.direction == "flat"
+    return fallback if trend.nil? || trend.direction.flat?
 
     bg_class, text_class, symbol, icon = case trend.direction
     when "up"

app/models/time_series.rb

@@ -1,20 +1,21 @@
 class TimeSeries
-  attr_reader :type
+  DIRECTIONS = %w[ up down ].freeze
 
-  def self.from_collection(collection, value_method, options = {})
-    data = collection.map do |obj|
-      { date: obj.date, value: obj.public_send(value_method), original: obj }
-    end
-    new(data, options)
+  attr_reader :values, :favorable_direction
+
+  def self.from_collection(collection, value_method)
+    collection.map do |obj|
+      {
+        date: obj.date,
+        value: obj.public_send(value_method),
+        original: obj
+      }
+    end.then { |data| new(data) }
   end
 
-  def initialize(data, options = {})
-    @type = options[:type] || :normal
-    initialize_series_data(data)
-  end
-
-  def values
-    @values ||= add_trends_to_series
+  def initialize(data, favorable_direction: "up")
+    @favorable_direction = (favorable_direction.presence_in(DIRECTIONS) || "up").inquiry
+    @values = initialize_values data
   end
 
   def first
@@ -30,53 +31,27 @@ class TimeSeries
   end
 
   def trend
-    TimeSeries::Trend.new(
+    TimeSeries::Trend.new \
       current: last&.value,
       previous: first&.value,
-      type: @type
-    )
+      series: self
   end
 
-  # Data shape that frontend expects for D3 charts
-  def to_json(*_args)
+  # `to_json` returns the data shape used by D3 charts
+  def to_json
     {
-      values: values.map do |v|
-        {
-          date: v.date,
-          value: JSON.parse(v.value.to_json),
-          trend: {
-            type: v.trend.type,
-            direction: v.trend.direction,
-            value: JSON.parse(v.trend.value.to_json),
-            percent: v.trend.percent
-          }
-        }
-      end,
-      trend: {
-        type: @type,
-        direction: trend.direction,
-        value: JSON.parse(trend.value.to_json),
-        percent: trend.percent
-      },
-      type: @type
+      values: values.map(&:as_json),
+      trend: trend.as_json,
+      favorable_direction: favorable_direction
     }.to_json
   end
 
   private
-    def initialize_series_data(data)
-      @series_data = data.nil? || data.empty? ? [] : data.map { |d| TimeSeries::Value.new(d) }.sort_by(&:date)
-    end
-
-    def add_trends_to_series
-      [ nil, *@series_data ].each_cons(2).map do |previous, current|
-        unless current.trend
-          current.trend = TimeSeries::Trend.new(
-            current: current.value,
-            previous: previous&.value,
-            type: @type
-          )
-        end
-        current
+    def initialize_values(data)
+      [ nil, *data ].each_cons(2).map do |previous, current|
+        TimeSeries::Value.new current,
+          previous: (TimeSeries::Value.new(previous) if previous),
+          series: self
       end
     end
 end

app/models/time_series/trend.rb

@@ -1,48 +1,95 @@
 class TimeSeries::Trend
-  attr_reader :type
+  include ActiveModel::Validations
 
-  # Tells us whether an increasing/decreasing trend is good or bad (i.e. a liability decreasing is good)
-  TYPES = %i[normal inverse].freeze
+  attr_reader :current, :previous
 
-  def initialize(current: nil, previous: nil, type: :normal)
-    validate_data_types(current, previous)
-    validate_type(type)
-    @current = current
+  delegate :favorable_direction, to: :series
+
+  validate :values_must_be_of_same_type, :values_must_be_of_known_type
+
+  def initialize(current:, previous:, series: nil)
+    @current = current || 0
     @previous = previous
-    @type = type
+    @series = series
+
+    validate!
   end
 
   def direction
-    return "flat" if @previous.nil? || @current == @previous
-    return "up" if @current && @current > @previous
-    "down"
+    if previous.nil? || current == previous
+      "flat"
+    elsif current && current > previous
+      "up"
+    else
+      "down"
+    end.inquiry
   end
 
   def value
-    return Money.new(0) if @previous.nil? && @current.is_a?(Money)
-    return 0 if @previous.nil?
-    @current - @previous
+    if previous.nil? && current.is_a?(Money)
+      Money.new 0
+    elsif previous.nil?
+      0
+    else
+      current - previous
+    end
   end
 
   def percent
-    return 0.0 if @previous.nil?
-    return Float::INFINITY if @previous == 0
-    ((extract_numeric(@current) - extract_numeric(@previous)).abs / extract_numeric(@previous).abs.to_f * 100).round(1).to_f
+    if previous.nil?
+      0.0
+    elsif previous == 0
+      Float::INFINITY
+    else
+      change = (current_amount - previous_amount).abs
+      base = previous_amount.abs.to_f
+
+      (change / base * 100).round(1).to_f
+    end
+  end
+
+  def as_json
+    {
+      favorable_direction: favorable_direction,
+      direction: direction,
+      value: value,
+      percent: percent
+    }.as_json
   end
 
   private
-    def validate_type(type)
-      raise ArgumentError, "Invalid type" unless TYPES.include?(type)
+    attr_reader :series
+
+    def values_must_be_of_same_type
+      unless current.class == previous.class || previous.nil?
+        errors.add :current, "must be of the same type as previous"
+        errors.add :previous, "must be of the same type as current"
+      end
     end
 
-    def validate_data_types(current, previous)
-      return if previous.nil? || current.nil?
-      raise ArgumentError, "Current and previous values must be of the same type" unless current.class == previous.class
-      raise ArgumentError, "Current and previous values must be of type Money or Numeric" unless current.is_a?(Money) || current.is_a?(Numeric)
+    def values_must_be_of_known_type
+      unless current.is_a?(Money) || current.is_a?(Numeric)
+        errors.add :current, "must be of type Money or Numeric"
+      end
+
+      unless previous.is_a?(Money) || previous.is_a?(Numeric) || previous.nil?
+        errors.add :previous, "must be of type Money, Numeric or nil"
+      end
+    end
+
+    def current_amount
+      extract_numeric current
+    end
+
+    def previous_amount
+      extract_numeric previous
     end
 
     def extract_numeric(obj)
-      return obj.amount if obj.is_a? Money
-      obj
+      if obj.is_a? Money
+        obj.amount
+      else
+        obj
+      end
     end
 end

app/models/time_series/value.rb

@@ -1,21 +1,18 @@
 class TimeSeries::Value
   include Comparable
+  include ActiveModel::Validations
 
-  attr_accessor :trend
   attr_reader :value, :date, :original
 
-  def initialize(obj)
-    @original = obj.fetch(:original, obj)
+  validates :date, presence: true
+  validate :value_must_be_of_known_type
 
-    if obj.is_a?(Hash)
-      @date = obj[:date]
-      @value = obj[:value]
-    else
-      @date = obj.date
-      @value = obj.value
-    end
+  def initialize(obj, series: nil, previous: nil)
+    @date, @value, @original = parse_object obj
+    @series = series
+    @trend = create_trend previous
 
-    validate_input
+    validate!
   end
 
   def <=>(other)
@@ -24,9 +21,41 @@ class TimeSeries::Value
     result
   end
 
+  def as_json
+    {
+      date: date,
+      value: value.as_json,
+      trend: trend.as_json
+    }
+  end
+
   private
-    def validate_input
-      raise ArgumentError, "Date is required" unless @date.is_a?(Date)
-      raise ArgumentError, "Money or Numeric value is required" unless @value.is_a?(Money) || @value.is_a?(Numeric)
+    attr_reader :series, :trend
+
+    def parse_object(obj)
+      if obj.is_a?(Hash)
+        date = obj[:date]
+        value = obj[:value]
+        original = obj.fetch(:original, obj)
+      else
+        date = obj.date
+        value = obj.value
+        original = obj
+      end
+
+      [ date, value, original ]
+    end
+
+    def create_trend(previous)
+      TimeSeries::Trend.new \
+        current: value,
+        previous: previous&.value,
+        series: series
+    end
+
+    def value_must_be_of_known_type
+      unless value.is_a?(Money) || value.is_a?(Numeric)
+        errors.add :value, "must be a Money or Numeric"
+      end
     end
 end

app/views/shared/_trend_change.html.erb

@@ -1,7 +1,7 @@
 <%# locals: { trend: } %>
 <% styles = trend_styles(trend) %>
 <p class="text-sm <%= styles[:text_class] %>">
-  <% if trend.direction == "flat" %>
+  <% if trend.direction.flat? %>
     <span>No change</span>
   <% else %>
     <span><%= styles[:symbol] %><%= trend.value.is_a?(Money) ? format_money(trend.value.abs) : trend.value.abs %></span>

app/views/shared/_value_heading.html.erb

@@ -16,7 +16,7 @@
   </p>
   <% if trend.nil? %>
     <p class="text-sm text-gray-500">Data not available for the selected period</p>
-  <% elsif trend.direction == "flat" %>
+  <% elsif trend.direction.flat? %>
     <p class="text-sm text-gray-500">No change vs. prior period</p>
   <% else %>
     <div class="flex items-center gap-2">

test/models/family_test.rb

@@ -73,9 +73,9 @@ class FamilyTest < ActiveSupport::TestCase
     assert_equal @expected_snapshots.count, net_worth_series.values.count
 
     @expected_snapshots.each_with_index do |row, index|
-      expected_assets = TimeSeries::Value.new(date: row["date"], value: Money.new(row["assets"].to_d))
-      expected_liabilities = TimeSeries::Value.new(date: row["date"], value: Money.new(row["liabilities"].to_d))
-      expected_net_worth = TimeSeries::Value.new(date: row["date"], value: Money.new(row["net_worth"].to_d))
+      expected_assets = TimeSeries::Value.new({ date: row["date"], value: Money.new(row["assets"].to_d) })
+      expected_liabilities = TimeSeries::Value.new({ date: row["date"], value: Money.new(row["liabilities"].to_d) })
+      expected_net_worth = TimeSeries::Value.new({ date: row["date"], value: Money.new(row["net_worth"].to_d) })
 
       assert_in_delta expected_assets.value.amount, Money.new(asset_series.values[index].value).amount, 0.01
       assert_in_delta expected_liabilities.value.amount, Money.new(liability_series.values[index].value).amount, 0.01

test/models/time_series_test.rb

@@ -6,7 +6,7 @@ class TimeSeriesTest < ActiveSupport::TestCase
 
     assert_equal Money.new(100), series.first.value
     assert_equal Money.new(200), series.last.value
-    assert_equal :normal, series.type
+    assert_equal "up", series.favorable_direction
     assert_equal "up", series.trend.direction
     assert_equal Money.new(100), series.trend.value
     assert_equal 100.0, series.trend.percent
@@ -18,21 +18,18 @@ class TimeSeriesTest < ActiveSupport::TestCase
     assert_equal 100, series.first.value
     assert_equal 200, series.last.value
     assert_equal 100, series.on(1.day.ago.to_date).value
-    assert_equal :normal, series.type
+    assert_equal "up", series.favorable_direction
     assert_equal "up", series.trend.direction
     assert_equal 100, series.trend.value
     assert_equal 100.0, series.trend.percent
   end
 
-  test "when nil or empty array passed, it returns empty series" do
-    series = TimeSeries.new(nil)
-    assert_equal [], series.values
-
+  test "when empty array passed, it returns empty series" do
     series = TimeSeries.new([])
 
     assert_nil series.first
     assert_nil series.last
-    assert_equal({ values: [], trend: { type: "normal", direction: "flat", value: 0, percent: 0.0 }, type: "normal" }.to_json, series.to_json)
+    assert_equal({ values: [], trend: { favorable_direction: "up", direction: "flat", value: 0, percent: 0.0 }, favorable_direction: "up" }.to_json, series.to_json)
   end
 
   test "money series can be serialized to json" do
@@ -41,16 +38,16 @@ class TimeSeriesTest < ActiveSupport::TestCase
         {
           date: 1.day.ago.to_date,
           value: { amount: "100.0", currency: "USD" },
-          trend: { type: "normal", direction: "flat", value: { amount: "0.0", currency: "USD" }, percent: 0.0 }
+          trend: { favorable_direction: "up", direction: "flat", value: { amount: "0.0", currency: "USD" }, percent: 0.0 }
         },
         {
           date: Date.current,
           value: { amount: "200.0", currency: "USD" },
-          trend: { type: "normal", direction: "up", value: { amount: "100.0", currency: "USD" }, percent: 100.0 }
+          trend: { favorable_direction: "up", direction: "up", value: { amount: "100.0", currency: "USD" }, percent: 100.0 }
         }
       ],
-      trend: { type: "normal", direction: "up", value: { amount: "100.0", currency: "USD" }, percent: 100.0 },
-      type: "normal"
+      trend: { favorable_direction: "up", direction: "up", value: { amount: "100.0", currency: "USD" }, percent: 100.0 },
+      favorable_direction: "up"
     }.to_json
 
     series = TimeSeries.new([ { date: 1.day.ago.to_date, value: Money.new(100) }, { date: Date.current, value: Money.new(200) } ])
@@ -61,11 +58,11 @@ class TimeSeriesTest < ActiveSupport::TestCase
   test "numeric series can be serialized to json" do
     expected_values = {
       values: [
-        { date: 1.day.ago.to_date, value: 100, trend: { type: "normal", direction: "flat", value: 0, percent: 0.0 } },
-        { date: Date.current, value: 200, trend: { type: "normal", direction: "up", value: 100, percent: 100.0 } }
+        { date: 1.day.ago.to_date, value: 100, trend: { favorable_direction: "up", direction: "flat", value: 0, percent: 0.0 } },
+        { date: Date.current, value: 200, trend: { favorable_direction: "up", direction: "up", value: 100, percent: 100.0 } }
       ],
-      trend: { type: "normal", direction: "up", value: 100, percent: 100.0 },
-      type: "normal"
+      trend: { favorable_direction: "up", direction: "up", value: 100, percent: 100.0 },
+      favorable_direction: "up"
     }.to_json
 
     series = TimeSeries.new([ { date: 1.day.ago.to_date, value: 100 }, { date: Date.current, value: 200 } ])