CODEWITHSUNDEEP
c#dependency-injection
shamp00
shamp00

Reputation: 11326

How do I apply dependency injection to an abstract factory

I've just finished Mark Seemann's book Dependency Injection in .NET and I'm now trying to refactor some legacy code. (I am not, at this stage, relying on any particular DI container, rather just trying to move all the dependencies to one place).

I'm looking at the following factory class which determines the ArchiveType by reading the first few bytes of the archive with archiveReader.GetArchiveType() and then returns an instance of an ArchiveRestorer based on the ArchiveType enum.

public class ArchiveRestorerFactory : IArchiveRestorerFactory
{
    public ArchiveRestorer Create(ArchiveReader archiveReader)
    {
        ArchiveType type = archiveReader.GetArchiveType();
        switch (type)
        {
            case ArchiveType.CurrentData:
                return new CurrentDataArchiveRestorer(archiveReader);
                break;
            case ArchiveType.HistoricalData:
                return new HistoricalDataArchiveRestorer(archiveReader);
                break;
            case ArchiveType.AuditTrail:
                return new AuditTrailArchiveRestorer(archiveReader);
                break;
            default:
                throw new Exception("ArchiveRestorerFactory error: Unknown value for ArchiveType.");
        }
    }
}

How do I refactor this so that the class does not depend on the concrete types CurrentDataArchiveRestorer, HistoricalDataArchiveRestorer and AuditTrailArchiveRestorer?

Should I move the three concrete restorers into the factory's constructor? 

public ArchiveRestorer Create(ArchiveReader archiveReader, 
    ArchiveRestorer currentDataArchiveRestorer, 
    ArchiveRestorer historicalDataArchiveRestorer, 
    ArchiveRestorer auditTrailDataArchiveRestorer)
{
    // guard clauses...
    // assign to readonly fields
}

That seems to be the approach suggested here, but then it will instantiate all three restorers when only one is needed? What if I had 20 different possible concrete implementations instead?

I feel like I should be implementing a concrete factory for each type of restorer and returning that instead but then I would just be replacing one new with another.

What is the best way to refactor this?

Upvotes: 5

Views: 2771

Answers (5)

Danny van der Kraan
Danny van der Kraan

Reputation: 5366

I would solve this problem by agreeing on a naming convention and utilizing Unity's ability to name registrations. Example of this here: https://dannyvanderkraan.wordpress.com/2015/06/29/real-world-example-of-dependency-injection-based-on-run-time-values/

Upvotes: 0

Mark Seemann
Mark Seemann

Reputation: 233387

Why not make the ArchiveReader responsible for creating the appropriate ArchiveRestorer? Then the first iteration of the code would look like this:

public class ArchiveRestorerFactory : IArchiveRestorerFactory
{
    public ArchiveRestorer Create(ArchiveReader archiveReader)
    {
        ArchiveRestorer restorer = archiveReader.GetArchiveRestorer();
        return restorer;
    }
}

By then, it should be pretty obvious that the factory is redundant, so in the second iteration of the code you can throw it away let the consumers invoke the ArchiveReader directly.

Upvotes: 2

NoWar
NoWar

Reputation: 37652

interface ILogger
{
    void Log(string data);
}

class Logger : ILogger
{
    .
    .
    .
}

At this point, you use an intermediate factory object to return the logger to be used within the component:

class MyComponent
{
  void DoSomeWork()
  {
    // Get an instance of the logger
    ILogger logger = Helpers.GetLogger();
    // Get data to log
    string data = GetData();

    // Log
    logger.Log(data);
  }
}

class Helpers
{
  public static ILogger GetLogger()
  {
    // Here, use any sophisticated logic you like
    // to determine the right logger to instantiate.

    ILogger logger = null;
    if (UseDatabaseLogger)
    {
        logger = new DatabaseLogger();
    }
    else
    {
        logger = new FileLogger();
    }
    return logger;
  }
}
class FileLogger : ILogger
{
    .
    .
    .
}

class DatabaseLogger : ILogger
{
    .
    .
    .
}

Upvotes: 0

Kamal Kr
Kamal Kr

Reputation: 697

Make one interface with one method with returns type of that interface and let three archiver classes implement that interface and then in the create method the parameter type would be just the interface and it will return the required object by calling the method of interface you just created. So you don't need concrete type in create method.

Upvotes: 1

Jamie Dixon
Jamie Dixon

Reputation: 54021

The way I'd do this, given the code you've already got, would be to create a factory for each of these objects which has a Create() method.

I'd have an interface for these factories also and have them inherit from a general factory interface.

You can then use the interfaces as a point for injection into your constructor.

Which would be called similar to this:

case ArchiveType.CurrentData:
                return _currentDateArchiveRestorerFactory.Create(archiveReader);
                break;

Alternatively, it might be better to have a single factory that creates an instance of a given type. Since all of these objects are restorers you could just create the instance based on the enum rather than a switch.

_restorerFactory.Create(ArchiveType.CurrentData);

Upvotes: 2

Related Questions