Breaking SOLID Principles in multiple implementation of an Interface

Question

I am facing a problem with dependency inversion in a factory method and it is also breaking Open Closed principle. My code looks like below codes

    public interface IWriter
    {
        void WriteToStorage(string data);
    }

    public class FileWriter : IWriter
    {
        public void WriteToStorage(string data)
        {
            //write to file
        }
    }

    public class DBWriter : IWriter
    {
        public void WriteToStorage(string data)
        {
            //write to DB
        }
    }

Now I an using a factory class to solve the object creation. It look like below code

public interface IFactory
{
    IWriter GetType(string outputType);
}

public class Factory : IFactory
{
    public IWriter GetType(string outputType)
    {
        IWriter writer = null;
        if (outputType.Equels("db"))
        {
            writer = new FileWriter();
        }
        else if (outputType.Equels("db"))
        {
            writer = new DBWriter();
        }
    }
}

Now the problem is the Factory class is breaking Open closed principle so it also breakes Dependency Inversion Principle

And then

public interface ISaveDataFlow
{
    void SaveData(string data, string outputType);
}

public class SaveDataFlow : ISaveDataFlow
{
    private IFactory _writerFactory = null;
    public SaveDataFlow(IFactory writerFactory)
    {
        _writerFactory = writerFactory;
    }
    public void SaveData(string data, string outputType)
    {
        IWriter writer = _writerFactory.GetType(outputType);
        writer.WriteToStorage(data);
    }
}

As the above factory class is breaking the dependency inversion I remove the Factory class and change the SaveDataFlow class like below

public class SaveDataFlow : ISaveDataFlow
{
    private IWriter _dbWriter = null;
    private IWriter _fileWriter = null;
    public SaveDataFlow([Dependency("DB")]IWriter dbWriter,
                        [Dependency("FILE")]IWriter fileWriter)
    {
        _dbWriter = dbWriter;
        _fileWriter = fileWriter;
    }
    public void SaveData(string data, string outputType)
    {
        if (outputType.Equals("DB"))
        {
            _dbWriter.WriteToStorage(data);
        }
        else if (outputType.Equals("FILE"))
        {
            _fileWriter.WriteToStorage(data);
        }
    }
}

And resolved those dependencies using Unity Framework

container.RegisterType<IWriter, DBWriter>("DB");
container.RegisterType<IWriter, FileWriter>("FILE");

Yet eventually I am ending up breaking Open Closed Principle. I need a better design/solution to solve such a problem yet I must follow SOLID Principles.

Answer 1

I would simply turn it into a strategy pattern:

namespace UnityMutliTest
{
    using System;
    using System.Collections.Generic;
    using System.Linq;

    using Microsoft.Practices.Unity;

    class Program
    {
        static void Main(string[] args)
        {
            IUnityContainer container = new UnityContainer();

            container.RegisterType<IWriter, FileWriter>("file");
            container.RegisterType<IWriter, DbWriter>("db");

            container.RegisterType<IWriterSelector, WriterSelector>();

            var writerSelector = container.Resolve<IWriterSelector>();

            var writer = writerSelector.SelectWriter("FILE");

            writer.Write("Write me data");

            Console.WriteLine("Success");

            Console.ReadKey();
        }
    }

    interface IWriterSelector
    {
        IWriter SelectWriter(string output);
    }

    class WriterSelector : IWriterSelector
    {
        private readonly IEnumerable<IWriter> writers;

        public WriterSelector(IWriter[] writers)
        {
            this.writers = writers;
        }

        public IWriter SelectWriter(string output)
        {
            var writer = this.writers.FirstOrDefault(x => x.CanWrite(output));

            if (writer == null)
            {
                throw new NotImplementedException($"Couldn't find a writer for {output}");
            }

            return writer;
        }
    }

    interface IWriter
    {
        bool CanWrite(string output);

        void Write(string data);
    }

    class FileWriter : IWriter
    {
        public bool CanWrite(string output)
        {
            return output == "FILE";
        }

        public void Write(string data)
        {
        }
    }

    class DbWriter : IWriter
    {
        public bool CanWrite(string output)
        {
            return output == "DB";
        }

        public void Write(string data)
        {
        }
    }
}

You can have as many IWriters as you want, just register them:

container.RegisterType<IWriter, LogWriter>("log");

You can even implement decorators over the writers if you want as well.

You use the (badly named) IWriterSelector as the implementation on how to select your writer, this should be concerned with only getting a writer! The throw exception here is really useful, it will fail fast if there is no implementation that suits your needs!!

If you ever have Open Closed problems, either use Strategy or Template patterns to overcome.

I use this pattern all the time, to great effect.

I've created a little extension method to prevent you having to name your instances:

static class UnityExtensions
{
    public static void RegisterMultipleType<TInterface, TConcrete>(this IUnityContainer container)
    {
        var typeToBind = typeof(TConcrete);
        container.RegisterType(typeof(TInterface), typeToBind, typeToBind.Name);
    }
}

container.RegisterMultipleType<IWriter, FileWriter>();

Answer 2

Solution 1

Choose before instantiation and use scopes

using(var scope = new Scope(unity))
{
    scope.register<IWriter, ConcreteWriter>();
    var flow = scope.Resolve<ISaveDataFlow>();

}

Solution 2

Inject your strategy at runtime.

ISaveDataFlow flow = ....
IWriter writer = GetWriterBasedOnSomeCondition();
flow.SaveData(data, writer);

I suspect that solution 2 is closer to what you are trying to achieve. Remember, you don't need to pass around a string to describe the strategy you want to use.

You can instead pass around the actual strategy you want to use, in this case, the actual IWriter, you want to use.

Then what you can do instead is have metadata on each IWriter to help the user choose which IWriter to use.

For example

public interface IWriter
{
   void WriteData(data);
   string Name {get;}
}

void GetWriterBasedOnSomeCondition()
{
    Dictionary<string, IWriter> writers = ...ToDictionary(x => x.Name);
    var choice = Console.ReadLine();
    return writers[choice];
}

Answer 3

I tend to use one of these approaches.

1. Break into different interfaces

public interface IWriter
{
    void WriteToStorage(string data);
}

public interface IFileWriter : IWriter
{
}

public interface IDBWriter: IWriter
{
}

public class FileWriter : IFileWriter 
{
    public void WriteToStorage(string data)
    {
        //write to file
    }
}

public class DBWriter : IDBWriter
{
    public void WriteToStorage(string data)
    {
        //write to DB
    }
}

Pros: You can inject the correct implementation based on the interface, which doesn't break the OCP.

Cons: You have empty interfaces.

---

2. Use an enum to separate them (strategy pattern)

public interface IWriter
{
    void WriteToStorage(string data);
    StorageType WritesTo { get; }
}

public enum StorageType 
{
    Db = 1,
    File = 2
}

public class Factory : IFactory
{
    public IEnumerable<IWriter> _writers;

    public Factory(IWriter[] writers)
    {
        _writers = writers;
    }

    public IWriter GetType(StorageType outputType)
    {
        IWriter writer = _writers.FirstOrDefault(x => x.WritesTo == outputType);
        return writer;
    }
}

Pros: You can inject them both and then use the one you want by using the enum.

Cons: I guess it kinda breaks the OCP-principle the same way as in your first example.

More about the strategy pattern in this excellent answer from Mark Seemann.

---

3. Build a factory that creates items based on a func.

In your registration:

container.RegisterType<IWriter, DBWriter>("DB");
container.RegisterType<IWriter, FileWriter>("FILE");
container.RegisterType<IFactory, Factory>(
    new ContainerControlledLifetimeManager(),
    new InjectionConstructor(
        new Func<string, IWriter>(
            writesTo => container.Resolve<IWriter>(writesTo));

And your factory

public class Factory : IFactory
{
    private readonly Func<string, IWriter> _createFunc;

    public Factory(Func<string, IWriter> createFunc)
    {
        _createFunc = createFunc;
    }

    public IWriter CreateScope(string writesTo)
    {
        return _createFunc(writesTo);
    }
}

Pros: Moves the entire dependency to the registration.

Cons: A wrapper for a service-locator pattern. Can be a bit hard to read.

---

None of the examples above is perfect, as each of them has their pros and cons.

Similiar question here: Inject require object depends on condition in constructor injection

Answer 4

In .NET Core (it's not clear from the question what framework is being used), you can use the built-in DI to achieve the strategy pattern quite easily with very little code.

In Startup.ConfigureServices:

services
    .AddScoped<IWriter, FileWriter>()
    .AddScoped<IWriter, DBWriter>()
    .AddScoped<ISaveDataFlow, SaveDataFlow>();

Add an method to IWriter for the strategy algorithm:

public interface IWriter
{
    bool CanWrite(string outputType);
    void WriteToStorage(string data);
}

public class FileWriter : IWriter
{
    bool CanWrite(string outputType) => outputType == "FILE";
    public void WriteToStorage(string data) {}
}

public class DBWriter : IWriter
{
    bool CanWrite(string outputType) => outputType == "DB";
    public void WriteToStorage(string data) {}
}

Then change the constructor of SaveDataFlow to use a collection type, and change SaveData to call the algorithm method of all resolved IWriter types.

public class SaveDataFlow : ISaveDataFlow
{
    private readonly IWriter _writers;

    public SaveDataFlow(IEnumerable<IWriter> writers)
    {
        _writers= writers;
    }

    public void SaveData(string data, string outputType)
    {
        _writers.Single(w => w.CanWrite(outputType)).WriteToStorage(data);
    }
}

This now complies with the Open/Closed Principle as the concrete selection is only within the concrete classes themselves.