Breaking SOLID Principles in multiple implementati

问题:

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.

回答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>();

回答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];
}

回答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