Is it ok to fire events from Dispose()?

Question

In my current project I'm using classes which implement the following ITransaction interface shown below. This is a generic interface for a transaction that can be undone. I also have a TransactionSet class which is used to attempt multiple Transactions or TransactionSets, and can ultimately be used to create a tree of transactions.

Some implementations of ITransaction keep temporary references to object instances or files it may use later if there is a call to Undo(). A successful transaction can later be confirmed after which Undo() is no longer allowed and there is thus also no longer a need for the temporary data. Currently I'm using Dispose() as my confirmation method to clean up any temporary resources.

However, now I'd like my transactions to also fire events to notify other classes of what has taken place. I do not want the events to fire unless the transaction is confirmed. Because I don't want to allow a transaction to fire events multiple times by being undone and then run again.

Since I'm using Dispose() to confirm a transaction is there anything wrong with also firing these events from it? Or would it be better to have a separate Confirm() method on my interface that fires the events in addition to Dispose() which cleans up the temporary data? I can't think of any case where I would want to confirm, but not dispose a transaction. Yet it's not entirely clear to me what I should and should not do within Dispose().

public enum TransactionStatus
{
    NotRun, // the Transaction has not been run, or has been undoed back to the original state
    Successful, ///the action has been run and was successful
    Error //there was an attempt to run the action but it failed
}

/// <summary>
/// Generic transaction interface
/// </summary>
public interface ITransaction
{
    TransactionStatus Status { get; }

    /// <summary>
    /// Attempts the transaction returns true if successful, false if failed.
    /// If failed it is expected that everything will be returned to the original state.
    /// Does nothing if status is already Successful
    /// </summary>
    /// <returns></returns>
    bool Go();

    /// <summary>
    /// Reverts the transaction
    /// Only does something if status is successful.
    /// Should return status to NotRun
    /// </summary>
    void Undo();

    /// <summary>
    /// A message describing the cause of the error if Status == Error
    /// Otherwise equal String.Empty
    /// </summary>
    string ErrorMessage { get; }
}

Answer 1

Dispose is not a special method - it's not like a ctor or a finalizer or anything - it's just a helpful pattern to notify an object the consumer is done using it. There's no reason it can't raise events.

Answer 2

Dispose should simply clean up. I would implement Confirm() and Rollback() methods, if dispose gets called without either of them being called first it's an error that should at least be logged.

Answer 3

IDisposable is simply a runtime-integrated design pattern that facilitates object cleanup in a more efficient manner than finalization. There is very little you "can't" do in a disposal method, however you should be wary of doing some things.

While the IDisposable.Dispose() method is not a "real" destructor or finalizer, it can adversely affect the lifetime of an object if other objects maintain (or perhaps even take) a reference to the disposing object during disposal events. If you are careful about how you implement such a system, you can mitigate the possible side effects. However, it is important to realize the potential that such an implementation offers...such as increased attack surface for a malicious coder to exploit by, say, keeping your transaction objects alive indefinitely.

Answer 4

For sure, you can fire any events in Dispose method. However, if you want to fire events to confirm transaction being there, I think you should have a separate method to fire events. Dispose() is the way to clean up internal resources or dispose internal instances as a well-known pattern. After disposed, your transaction install should be not be there or to be used any more. Therefore, you may consider a separate method to as confirmation that the temporary will not be available, with flag or status in Transaction to indicate that.