Devo refactoring questa applicazione di rotaie?

Question

Devo refactoring questa applicazione di rotaie?

#1 da (2 voti)

4

Ho preso il codice base per un'applicazione ruby-on-rails che si affida molto ai callback di ActiveRecord per eseguire le regole di dominio.

L'applicazione può essere confrontata con un'applicazione bancaria, in cui un cliente ha un account. Su ogni account è possibile registrare una Transazione (una somma di denaro che viene inserita o rimossa dall'account). L'account ha un saldo_bancario che viene aggiornato ogni volta che viene creata una transazione. Questo aggiornamento avviene come un callback di ActiveRecord.

class Account < ActiveRecord::Base

  has_many :transactions

  def update_account_balance
    account_balance = 0
    self.transaction_logs.each do |transaction|
      account_balance = account_balance + (transaction.amount + transaction.commission)
    end
    self.account_balance = account_balance
  end
end

class Transaction < ActiveRecord::Base

  belongs_to :account

  after_save :update_account_value

  def update_account_value
    self.account.update_account_balance
    self.account.save!
  end
end

Trovo che questo modo di codificare rende molto difficile capire cosa succede (e cosa succede quando si verifica un errore). Inoltre, poiché l'account deve caricare tutte le transazioni, questo sarà più pesante man mano che vengono aggiunte sempre più transazioni.

Se avessi sviluppato un'applicazione come questa in .NET (in cui sono molto più familiare) avrei creato qualcosa del genere:

public class Account {

  public void AddTransaction(decimal amount) {
    this.Transactions.Add(new Transaction(amount));
    this.AccountValue += amount;
  }
}

Quindi assicurati che un oggetto Transaction sia immutabile.

Sono molto tentato di ridefinire il sistema in qualcosa del genere. Ma dovrei? Sarebbe il "modo rotaie"?

L'inconveniente, per quanto posso dire, è che il mio controller deve gestire in modo esplicito il controllo delle transazioni del database, cosa che è accaduta implicitamente nel Transaction.save! chiamata attualmente chiamata nell'applicazione. Ci sono altri fattori di cui dovrei essere a conoscenza?

refactoring ruby-on-rails

posta Pete 14.01.2012 - 17:27

fonte

1 risposta

Leggi altre domande sui tag refactoring ruby-on-rails

Si può utilizzare Foswiki come sostituto Redmine distribuito? Architettura generale con tabelle codici?

score 2 · Accepted Answer

La logica dell'applicazione esistente non è terribile. Non apporterei alcuna modifica per spostare la logica nel controller; questa è la direzione opposta rispetto alla metodologia preferita di spingere la logica nel modello il più possibile.

L'unica cosa che mi sembra potenzialmente (o forse inevitabilmente) pericolosa è che, come hai detto, Account#update_account_balance sta iterando sulla transazione ogni per trovare quale dovrebbe essere il suo equilibrio, < em> ogni tempo viene salvata una nuova transazione. Questo non è solo un potenziale collo di bottiglia per le prestazioni, ma anche una potenziale condizione di competizione, se due transazioni vengono salvate quasi alla stessa ora e una è esclusa dal set restituito dalla chiamata a Account#transaction_logs .

Il mio refactoring suggerito:

class Account < ActiveRecord::Base
  attr_protected :account_balance

  has_many :transactions

  def update_account_balance(transaction)
    self.account_balance = (self.account_balance + transaction.amount + transaction.commission)
    self.save!
  end

end

class Transaction < ActiveRecord::Base
  attr_readonly :amount, :commission

  belongs_to :account

end

class TransactionObserver < ActiveRecord::Observer

  def after_create(transaction)
    transaction.account.update_account_balance(transaction)
  end

end

L'uso di ActiveRecord :: Observer qui ti aiuta a seguire il Principio di Responsabilità Unica; l'aggiornamento di un Account non fa parte della responsabilità principale di un Transaction , ma deve verificarsi ogni volta che un Transaction viene mantenuto.

L'uso di attr_readonly in Transaction protegge i valori dalle operazioni di aggiornamento e attr_protected impedisce l'assegnazione di massa su :account_balance .