C# - Should an object be responsible for creating a history object when it changes something like status?

Question

This is more of an architecture/best practices question than anything else, so please feel free to add your two cents. I know i stated status in the title, but this goes for any basic property of an object. I think the account example below will help demonstrate my question a little better than status.

Here is a sample Account object:

public class Account
{
   private IList<Transaction> _transactions;

   public AddTransaction(trans as Transaction)
   {
      _transaction.add(trans)
   }
}

Now lets say I want to start keeping a history of every time a transaction is added with this object.

public class AccountHistory
{
   private DateTime _historyDate;
   private String _details;

   public AccountHistory(string details)
   {
      _historyDate = DateTime.Now;
      _details = details;
   }
}

At this level what I would normally do is add a collection of history events to the account object and also add a line of code to create a history event inside of the AddTransaction() method like this

public AddTransaction(trans as Transaction)
{
   _transaction.add(trans);
   **_historyEvents.add(new AccountHistory("Transaction Added: " + trans.ToString());**
}

Now the next part is where the problem starts to arise. Suppose I want to do a bulk posting and I want to retain a record of which accounts were changed in this bulk posting for something like a report or if I needed to undo it later. So I would create an object like this.

public class HistoryGroup()
{
   private IList<AccountHistory> _events;
}

From here I see a few different options to handle this since it can't be handled by the example code above.

1) Create a function in a Service type object that loops through a list of accounts calling the AddTransaction() method and also creating history records tied to a HistoryGroup

 public void AddTransactions(IList<Account> accounts, Transaction trans)
    {
       HistoryGroup history = new HistoryGroup(); 
       for (int x=0;x <=accounts.Count - 1; x++)
       {
         accounts(x).AddTransaction(trans);
         history.AddEvent(new AccountHistory("Added Transaction: " + trans.ToString();
       }
    }

2) Pass some type of HistoryManager object into the AddTransaction method along with the transaction to be added. Then the function could use the history manager to create the records.

Ok this post is long enough. If i've not been clear enough let me know. Thanks for you input.

Answer 1

Your method might work just fine, but let me propose an alternative.

Why not add a TransactionAdded Event to the Account class.

You could then subscribe to the Event from (I'm guessing here) the HistoryGroup object so that a new AccountHistory object was added every time the Event fired.

UPDATE

As mentioned in the comments, another method of accomplishing the goal would be to have HistoryGroup implement an interface (ITransactionLogger or something similar) and then modify Account so that the ITransactionLogger dependency can be injected.

Going either of these routes makes things a little easier to manage from the complexity and debugging standpoint, but doesn't allow for multiple Loggers like Events.

That would make your code a little more flexible and at the same time allow other consumers interested in the TransactionAdded Event to subscribe.

Answer 2

I agree with Justin's answer in some ways, but one of the tags on the OP is POCO; adding an event to the Account class would in some ways un-POCO your POCO.

If you're into AOP and other such, you could use interception (most IoC frameworks, including Unity and Castle offer this functionality) to grab transactions of interest.

The benefit of interception is that your Account class has no coupling whatsoever with the AccountHistory class, the interception is highly configurable according to whatever rules you want, and it is easily changed without forcing an application recompile (if you put AccountHistory into a different assembly with the interception handlers). By using interception you are making your code more focused on the business domain rather on what could be considered an infrastructure task (auditing).

Again, this is another alternative for your toolbox; if you don't need to serialize your POCO's over the wire for any reason, then implementing the Observer Pattern (GoF) through events as suggested by Justin may be a more light-weight approach.

Answer 3

The gang of four seem to think so. Transactions, history tracking, and un-doing are all part of a command pattern contract. You can implement history with a stack. Here's a snippet of relevant code including the contract, note that not all methods are or have to be implemented:

public interface ICommand
{
    void execute();
    void undo();
    void store();
    void load();
}
public class ManagerMacro : ICommand
{
    List<ICommand> Commands;
    Stack commandStack;
    /// <summary>
    /// Use in combination with AddSteps
    /// </summary>
    //public ManagerMacro()
    //{

    //}
    public ManagerMacro(List<ICommand> commands)
    {
        this.Commands = commands;
        this.commandStack = new Stack();
    }

    #region ICommand Members

    public void execute()
    {
        for (int i = 0; i < Commands.Count; i++)
        {
            commandStack.Push(Commands[i]);
            Commands[i].execute();
        }
    }

    public void undo()
    {
        for (int i = 0; i < Commands.Count; i++)
        {
            if (commandStack.Count > 0)
            {
                ICommand Command = (ICommand)commandStack.Pop();
                Command.undo();
            }
        }
    }
    public void store()
    {
        throw new NotImplementedException();
    }

    public void load()
    {
        throw new NotImplementedException();
    }
    #endregion

    public void AddSteps(Steps[] steps)
    {
        foreach (Steps step in steps)
        {
            ICommand thisStep = null;
            switch (step)
            {
                case Steps.Manager1: thisStep = new Step1(); break;
                case Steps.Manager2: thisStep = new Step2(); break;
                case Steps.Manager3: thisStep = new Step3(); break;
                case Steps.Manager4: thisStep = new Step4(); break;
            }
            this.Commands.Add(thisStep);
        }
    }
}

Note that I also use a factory pattern.