How to accomplish mutable object refactor using dependency injection?

Question

There exists an "Audit" object that is used throughout the code base that I'm trying to refactor to allow for dependency injection, and eventually better unit testing. Up until this point I have had no problems creating interfaces for my classes, and injecting those through the constructor. This class however, is different. I see why/how it's different, but I'm not sure how to go about fixing it to work "properly".

Here is an example (dumbed down version, but the problem persists even in the example):

namespace ConsoleApplication1.test.DI.Original
{
    public class MultiUseDependencies
    {
        public MultiUseDependencies()
        {

        }

        public void Update()
        {
            Audit a = new Audit();
            a.preAuditValues = "Update";

            // if data already exists, delete it
            this.Delete();

            // Update values, implementation not important

            // Audit changes to the data
            a.AuditInformation();
        }

        public void Delete()
        {
            Audit a = new Audit();
            a.preAuditValues = "Delete";

            // Delete data, implementation omitted.

            a.AuditInformation();
        }
    }

    public class Audit
    {
        public string preAuditValues { get; set; }

        public void AuditInformation()
        {
            Console.WriteLine("Audited {0}", preAuditValues);
        }
    }
}

In the above, the Update function (implementation not shown) gets the "pre change" version of the data, deletes the data (and audits it), inserts/updates the changes to the data, then audits the insert/update.

If I were to run from a console app:

Console.WriteLine("\n");
test.DI.Original.MultiUseDependencies mud = new test.DI.Original.MultiUseDependencies();
mud.Update();

I would get:

Audited Delete

Audited Update

This is the expected behavior. Now in the way the class is implemented, I can already see there will be a problem, but I'm not sure how to correct it. See the (initial) refactor with DI:

namespace ConsoleApplication1.test.DI.Refactored
{
    public class MultiUseDependencies
    {

        private readonly IAudit _audit;

        public MultiUseDependencies(IAudit audit)
        {
            _audit = audit;
        }

        public void Update()
        {
            _audit.preAuditValues = "Update";

            // if data already exists, delete it
            this.Delete();

            // Update values, implementation not important

            // Audit changes to the data
            _audit.AuditInformation();
        }

        public void Delete()
        {
            _audit.preAuditValues = "Delete";

            // Delete data, implementation omitted.

            _audit.AuditInformation();
        }
    }

    public interface IAudit
    {
        string preAuditValues { get; set; }
        void AuditInformation();
    }

    public class Audit : IAudit
    {
        public string preAuditValues { get; set; }

        public void AuditInformation()
        {
            Console.WriteLine("Audited {0}", preAuditValues);
        }
    }
}

Running:

Console.WriteLine("\n");
test.DI.Refactored.MultiUseDependencies mudRefactored = new test.DI.Refactored.MultiUseDependencies(new test.DI.Refactored.Audit());
mudRefactored.Update();

I get (as expected, but incorrect):

Audited Delete

Audited Delete

The above is expected based on the implementation, but incorrect as per the original behavior. I'm not sure how exactly to proceed. The original implementation relies on distinct Audits to correctly keep track of what's changing. When I'm passing in the implementation of IAudit in the refactor, I am only getting a single instance of Audit, where the two are butting heads with each other.

Basically before the refactor, Audit is scoped on the function level. After the refactor, Audit is scoped on the class.

Is there an easy way to correct this?

Here's a fiddle with it in action: https://dotnetfiddle.net/YbpTm4

Answer 1

The problem is in your design. Audit is an object that is mutatable and that makes it runtime data. Injecting runtime data into the constructors of your components is an anti-pattern.

The solution is to change the design, for instance by defining an IAudit abstraction like this:

public interface IAuditHandler {
    void AuditInformation(string preAuditValues);
}

For this abstraction you can create the following implementation:

public class AuditHandler : IAuditHandler {
    public void AuditInformation(string preAuditValues) {
        var audit = new Audit();
        audit.preAuditValues = preAuditValues;
        audit.AuditInformation();
    }
}

The consumers can now depend on IAuditHandler:

public class MultiUseDependencies
{
    private readonly IAuditHandler _auditHandler;

    public MultiUseDependencies(IAuditHandler auditHandler) {
        _auditHandler = auditHandler;
    }

    public void Update() {
        this.Delete();

        _auditHandler.AuditInformation("Update");
    }

    public void Delete() {
        // Delete data, implementation omitted.

        _auditHandler.AuditInformation("Delete");
    }
}

But I should even take it a step further, because with your current approach you are polluting business code with cross-cutting concerns. The code for the audit trail is spread out and duplicated throughout your code base.

This however would be quite a change in your application's design, but would probably be very beneficial. You should definitely read this article to get an idea how you can improve your design this way.