From 64147758a3cc5bb0d053ef4f7902e4fc9b5ce043 Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Date: Sat, 10 May 2025 16:16:47 -0400
Subject: [PATCH] Remove bad abstraction

---
 app/models/balance/base_calculator.rb    | 35 -------------
 app/models/balance/forward_calculator.rb | 35 ++++++++++++-
 app/models/balance/reverse_calculator.rb | 35 ++++++++++++-
 app/models/holding/base_calculator.rb    | 62 ------------------------
 app/models/holding/forward_calculator.rb | 58 +++++++++++++++++++++-
 app/models/holding/reverse_calculator.rb | 54 ++++++++++++++++++++-
 6 files changed, 178 insertions(+), 101 deletions(-)
 delete mode 100644 app/models/balance/base_calculator.rb
 delete mode 100644 app/models/holding/base_calculator.rb

diff --git a/app/models/balance/base_calculator.rb b/app/models/balance/base_calculator.rb
deleted file mode 100644
index 2d01dfe7..00000000
--- a/app/models/balance/base_calculator.rb
+++ /dev/null
@@ -1,35 +0,0 @@
-class Balance::BaseCalculator
-  attr_reader :account
-
-  def initialize(account)
-    @account = account
-  end
-
-  def calculate
-    Rails.logger.tagged(self.class.name) do
-      calculate_balances
-    end
-  end
-
-  private
-    def sync_cache
-      @sync_cache ||= Balance::SyncCache.new(account)
-    end
-
-    def build_balance(date, cash_balance, holdings_value)
-      Balance.new(
-        account_id: account.id,
-        date: date,
-        balance: holdings_value + cash_balance,
-        cash_balance: cash_balance,
-        currency: account.currency
-      )
-    end
-
-    def calculate_next_balance(prior_balance, transactions, direction: :forward)
-      flows = transactions.sum(&:amount)
-      negated = direction == :forward ? account.asset? : account.liability?
-      flows *= -1 if negated
-      prior_balance + flows
-    end
-end
diff --git a/app/models/balance/forward_calculator.rb b/app/models/balance/forward_calculator.rb
index d024d2c6..4e6f2d5c 100644
--- a/app/models/balance/forward_calculator.rb
+++ b/app/models/balance/forward_calculator.rb
@@ -1,4 +1,16 @@
-class Balance::ForwardCalculator < Balance::BaseCalculator
+class Balance::ForwardCalculator
+  attr_reader :account
+
+  def initialize(account)
+    @account = account
+  end
+
+  def calculate
+    Rails.logger.tagged("Balance::ForwardCalculator") do
+      calculate_balances
+    end
+  end
+
   private
     def calculate_balances
       current_cash_balance = 0
@@ -25,4 +37,25 @@ class Balance::ForwardCalculator < Balance::BaseCalculator
 
       @balances
     end
+
+    def sync_cache
+      @sync_cache ||= Balance::SyncCache.new(account)
+    end
+
+    def build_balance(date, cash_balance, holdings_value)
+      Balance.new(
+        account_id: account.id,
+        date: date,
+        balance: holdings_value + cash_balance,
+        cash_balance: cash_balance,
+        currency: account.currency
+      )
+    end
+
+    def calculate_next_balance(prior_balance, transactions, direction: :forward)
+      flows = transactions.sum(&:amount)
+      negated = direction == :forward ? account.asset? : account.liability?
+      flows *= -1 if negated
+      prior_balance + flows
+    end
 end
diff --git a/app/models/balance/reverse_calculator.rb b/app/models/balance/reverse_calculator.rb
index 6a2ba70b..52a05608 100644
--- a/app/models/balance/reverse_calculator.rb
+++ b/app/models/balance/reverse_calculator.rb
@@ -1,4 +1,16 @@
-class Balance::ReverseCalculator < Balance::BaseCalculator
+class Balance::ReverseCalculator
+  attr_reader :account
+
+  def initialize(account)
+    @account = account
+  end
+
+  def calculate
+    Rails.logger.tagged("Balance::ReverseCalculator") do
+      calculate_balances
+    end
+  end
+
   private
     def calculate_balances
       current_cash_balance = account.cash_balance
@@ -35,4 +47,25 @@ class Balance::ReverseCalculator < Balance::BaseCalculator
 
       @balances
     end
+
+    def sync_cache
+      @sync_cache ||= Balance::SyncCache.new(account)
+    end
+
+    def build_balance(date, cash_balance, holdings_value)
+      Balance.new(
+        account_id: account.id,
+        date: date,
+        balance: holdings_value + cash_balance,
+        cash_balance: cash_balance,
+        currency: account.currency
+      )
+    end
+
+    def calculate_next_balance(prior_balance, transactions, direction: :forward)
+      flows = transactions.sum(&:amount)
+      negated = direction == :forward ? account.asset? : account.liability?
+      flows *= -1 if negated
+      prior_balance + flows
+    end
 end
diff --git a/app/models/holding/base_calculator.rb b/app/models/holding/base_calculator.rb
deleted file mode 100644
index d9b85d03..00000000
--- a/app/models/holding/base_calculator.rb
+++ /dev/null
@@ -1,62 +0,0 @@
-class Holding::BaseCalculator
-  attr_reader :account
-
-  def initialize(account)
-    @account = account
-  end
-
-  def calculate
-    Rails.logger.tagged(self.class.name) do
-      holdings = calculate_holdings
-      Holding.gapfill(holdings)
-    end
-  end
-
-  private
-    def portfolio_cache
-      @portfolio_cache ||= Holding::PortfolioCache.new(account)
-    end
-
-    def empty_portfolio
-      securities = portfolio_cache.get_securities
-      securities.each_with_object({}) { |security, hash| hash[security.id] = 0 }
-    end
-
-    def generate_starting_portfolio
-      empty_portfolio
-    end
-
-    def transform_portfolio(previous_portfolio, trade_entries, direction: :forward)
-      new_quantities = previous_portfolio.dup
-
-      trade_entries.each do |trade_entry|
-        trade = trade_entry.entryable
-        security_id = trade.security_id
-        qty_change = trade.qty
-        qty_change = qty_change * -1 if direction == :reverse
-        new_quantities[security_id] = (new_quantities[security_id] || 0) + qty_change
-      end
-
-      new_quantities
-    end
-
-    def build_holdings(portfolio, date, price_source: nil)
-      portfolio.map do |security_id, qty|
-        price = portfolio_cache.get_price(security_id, date, source: price_source)
-
-        if price.nil?
-          next
-        end
-
-        Holding.new(
-          account_id: account.id,
-          security_id: security_id,
-          date: date,
-          qty: qty,
-          price: price.price,
-          currency: price.currency,
-          amount: qty * price.price
-        )
-      end.compact
-    end
-end
diff --git a/app/models/holding/forward_calculator.rb b/app/models/holding/forward_calculator.rb
index d2f2e8d7..22759416 100644
--- a/app/models/holding/forward_calculator.rb
+++ b/app/models/holding/forward_calculator.rb
@@ -1,9 +1,65 @@
-class Holding::ForwardCalculator < Holding::BaseCalculator
+class Holding::ForwardCalculator
+  attr_reader :account
+
+  def initialize(account)
+    @account = account
+  end
+
+  def calculate
+    Rails.logger.tagged("Holding::ForwardCalculator") do
+      holdings = calculate_holdings
+      Holding.gapfill(holdings)
+    end
+  end
+
   private
     def portfolio_cache
       @portfolio_cache ||= Holding::PortfolioCache.new(account)
     end
 
+    def empty_portfolio
+      securities = portfolio_cache.get_securities
+      securities.each_with_object({}) { |security, hash| hash[security.id] = 0 }
+    end
+
+    def generate_starting_portfolio
+      empty_portfolio
+    end
+
+    def transform_portfolio(previous_portfolio, trade_entries, direction: :forward)
+      new_quantities = previous_portfolio.dup
+
+      trade_entries.each do |trade_entry|
+        trade = trade_entry.entryable
+        security_id = trade.security_id
+        qty_change = trade.qty
+        qty_change = qty_change * -1 if direction == :reverse
+        new_quantities[security_id] = (new_quantities[security_id] || 0) + qty_change
+      end
+
+      new_quantities
+    end
+
+    def build_holdings(portfolio, date, price_source: nil)
+      portfolio.map do |security_id, qty|
+        price = portfolio_cache.get_price(security_id, date, source: price_source)
+
+        if price.nil?
+          next
+        end
+
+        Holding.new(
+          account_id: account.id,
+          security_id: security_id,
+          date: date,
+          qty: qty,
+          price: price.price,
+          currency: price.currency,
+          amount: qty * price.price
+        )
+      end.compact
+    end
+
     def calculate_holdings
       current_portfolio = generate_starting_portfolio
       next_portfolio = {}
diff --git a/app/models/holding/reverse_calculator.rb b/app/models/holding/reverse_calculator.rb
index 62e2dc95..f52184d7 100644
--- a/app/models/holding/reverse_calculator.rb
+++ b/app/models/holding/reverse_calculator.rb
@@ -1,4 +1,17 @@
-class Holding::ReverseCalculator < Holding::BaseCalculator
+class Holding::ReverseCalculator
+  attr_reader :account
+
+  def initialize(account)
+    @account = account
+  end
+
+  def calculate
+    Rails.logger.tagged("Holding::ReverseCalculator") do
+      holdings = calculate_holdings
+      Holding.gapfill(holdings)
+    end
+  end
+
   private
     # Reverse calculators will use the existing holdings as a source of security ids and prices
     # since it is common for a provider to supply "current day" holdings but not all the historical
@@ -25,6 +38,11 @@ class Holding::ReverseCalculator < Holding::BaseCalculator
       holdings
     end
 
+    def empty_portfolio
+      securities = portfolio_cache.get_securities
+      securities.each_with_object({}) { |security, hash| hash[security.id] = 0 }
+    end
+
     # Since this is a reverse sync, we start with today's holdings
     def generate_starting_portfolio
       holding_quantities = empty_portfolio
@@ -37,4 +55,38 @@ class Holding::ReverseCalculator < Holding::BaseCalculator
 
       holding_quantities
     end
+
+    def transform_portfolio(previous_portfolio, trade_entries, direction: :forward)
+      new_quantities = previous_portfolio.dup
+
+      trade_entries.each do |trade_entry|
+        trade = trade_entry.entryable
+        security_id = trade.security_id
+        qty_change = trade.qty
+        qty_change = qty_change * -1 if direction == :reverse
+        new_quantities[security_id] = (new_quantities[security_id] || 0) + qty_change
+      end
+
+      new_quantities
+    end
+
+    def build_holdings(portfolio, date, price_source: nil)
+      portfolio.map do |security_id, qty|
+        price = portfolio_cache.get_price(security_id, date, source: price_source)
+
+        if price.nil?
+          next
+        end
+
+        Holding.new(
+          account_id: account.id,
+          security_id: security_id,
+          date: date,
+          qty: qty,
+          price: price.price,
+          currency: price.currency,
+          amount: qty * price.price
+        )
+      end.compact
+    end
 end