Search code examples
c#unit-testingdependency-injectionunity-containerfactory-pattern

How Can I Improve this Translator Object Factory to simplify unit testing?


In a project of mine, I have a number of classes based upon an ITranslator interface as follows:

interface ITranslator<TSource, TDest>
{
    TDest Translate(TSource toTranslate);
}

These classes translate a data object into a new form. To obtain an instance of a translator, I have an ITranslatorFactory with a method ITranslator<TSource, TDest> GetTranslator<TSource, TDest>(). I could not think of any way to store a collection of functions based upon a wide range of generics (only common ancestor here is Object), so the GetTranslator method is currently just using Unity to resolve an ITranslator<TSource, TDest> that matches the requested translator.

This implementation feels very awkward. I have read that the Service Locator is an anti-pattern, and regardless of if it is or is not, this implementation is making unit testing much more difficult because I must supply a configured Unity container to test any code which depends on the translator.

Unfortunately, I cannot think of a more effective strategy to obtain the proper translator. Does anyone have any suggestions on how I can refactor this setup into a more elegant solution?


Solution

  • Whether or not you agree with that service locator is anti-pattern, there are practical benefits to decoupling an application from the DI container that can't be ignored. There are edge cases where injecting the container into part of the application makes sense, but all other options should be exhausted before going that route.

    Option 1

    As StuartLC pointed out, it seems like you are reinventing the wheel here. There are many 3rd party implementations that already do translation between types. I would personally consider these alternatives to be the first choice and evaluate which option has the best DI support and whether it meets your other requirements.

    Option 2

    UPDATE

    When I first posted this answer, I didn't take into account the difficulties involved with using .NET Generics in the interface declaration of the translator with the Strategy pattern until I tried to implement it. Since Strategy pattern is still a possible option, I am leaving this answer in place. However, the end product I came up with isn't as elegant as I had first hoped - namely the implementations of the translators themselves are a bit awkward.

    Like all patterns, the Strategy pattern is not a silver bullet that works for every situation. There are 3 cases in particular where it is not a good fit.

    1. When you have classes that have no common abstract type (such as when using Generics in the interface declaration).
    2. When the number of implementations of the interface are so many that memory becomes an issue, since they are all loaded at the same time.
    3. When you must give the DI container control of the object lifetime, such as when you are dealing with expensive disposable dependencies.

    Maybe there is a way to fix the generic aspect of this solution and I hope someone else sees where I went wrong with the implementation and provides a better one.

    However, if you look at it entirely from a usage and testability standpoint (testability and awkwardness of use being the key problems of the OP), it is not that bad of a solution.

    The Strategy Pattern could be used to solve this very problem without injecting the DI container. This requires some re-plumbing to deal with the types you made generic, and a way to map the translator to use with the types involved.

    public interface ITranslator
    {
        Type SourceType { get; }
        Type DestinationType { get; }
        TDest Translate<TSource, TDest>(TSource toTranslate);
    }
    
    public static class ITranslatorExtensions
    {
        public static bool AppliesTo(this ITranslator translator, Type sourceType, Type destinationType)
        {
            return (translator.SourceType.Equals(sourceType) && translator.DestinationType.Equals(destinationType));
        }
    }
    

    We have a couple of objects that we want to translate between.

    class Model
    {
        public string Property1 { get; set; }
        public int Property2 { get; set; }
    }
    
    class ViewModel
    {
        public string Property1 { get; set; }
        public string Property2 { get; set; }
    }
    

    Then, we have our Translator implementations.

    public class ModelToViewModelTranslator : ITranslator
    {
        public Type SourceType
        {
            get { return typeof(Model); }
        }
    
        public Type DestinationType
        {
            get { return typeof(ViewModel); }
        }
    
        public TDest Translate<TSource, TDest>(TSource toTranslate)
        {
            Model source = toTranslate as Model;
            ViewModel destination = null;
            if (source != null)
            {
                destination = new ViewModel()
                {
                    Property1 = source.Property1,
                    Property2 = source.Property2.ToString()
                };
            }
    
            return (TDest)(object)destination;
        }
    }
    
    public class ViewModelToModelTranslator : ITranslator
    {
        public Type SourceType
        {
            get { return typeof(ViewModel); }
        }
    
        public Type DestinationType
        {
            get { return typeof(Model); }
        }
    
        public TDest Translate<TSource, TDest>(TSource toTranslate)
        {
            ViewModel source = toTranslate as ViewModel;
            Model destination = null;
            if (source != null)
            {
                destination = new Model()
                {
                    Property1 = source.Property1,
                    Property2 = int.Parse(source.Property2)
                };
            }
    
            return (TDest)(object)destination;
        }
    }
    

    Next, is the actual Strategy class that implements the Strategy pattern.

    public interface ITranslatorStrategy
    {
        TDest Translate<TSource, TDest>(TSource toTranslate);
    }
    
    public class TranslatorStrategy : ITranslatorStrategy
    {
        private readonly ITranslator[] translators;
    
        public TranslatorStrategy(ITranslator[] translators)
        {
            if (translators == null)
                throw new ArgumentNullException("translators");
    
            this.translators = translators;
        }
    
        private ITranslator GetTranslator(Type sourceType, Type destinationType)
        {
            var translator = this.translators.FirstOrDefault(x => x.AppliesTo(sourceType, destinationType));
            if (translator == null)
            {
                throw new Exception(string.Format(
                    "There is no translator for the specified type combination. Source: {0}, Destination: {1}.", 
                    sourceType.FullName, destinationType.FullName));
            }
            return translator;
        }
    
        public TDest Translate<TSource, TDest>(TSource toTranslate)
        {
            var translator = this.GetTranslator(typeof(TSource), typeof(TDest));
            return translator.Translate<TSource, TDest>(toTranslate);
        }
    }
    

    Usage

    using System;
    using System.Linq;
    using Microsoft.Practices.Unity;
    
    class Program
    {
        static void Main(string[] args)
        {
            // Begin Composition Root
            var container = new UnityContainer();
    
            // IMPORTANT: For Unity to resolve arrays, you MUST name the instances.
            container.RegisterType<ITranslator, ModelToViewModelTranslator>("ModelToViewModelTranslator");
            container.RegisterType<ITranslator, ViewModelToModelTranslator>("ViewModelToModelTranslator");
            container.RegisterType<ITranslatorStrategy, TranslatorStrategy>();
            container.RegisterType<ISomeService, SomeService>();
    
            // Instantiate a service
            var service = container.Resolve<ISomeService>();
    
            // End Composition Root
    
            // Do something with the service
            service.DoSomething();
        }
    }
    
    public interface ISomeService
    {
        void DoSomething();
    }
    
    public class SomeService : ISomeService
    {
        private readonly ITranslatorStrategy translatorStrategy;
    
        public SomeService(ITranslatorStrategy translatorStrategy)
        {
            if (translatorStrategy == null)
                throw new ArgumentNullException("translatorStrategy");
    
            this.translatorStrategy = translatorStrategy;
        }
    
        public void DoSomething()
        {
            // Create a Model
            Model model = new Model() { Property1 = "Hello", Property2 = 123 };
    
            // Translate to ViewModel
            ViewModel viewModel = this.translatorStrategy.Translate<Model, ViewModel>(model);
    
            // Translate back to Model
            Model model2 = this.translatorStrategy.Translate<ViewModel, Model>(viewModel);
        }
    }
    

    Note that if you copy each of the above code blocks (starting with the last one) into a console application, it will run as is.

    Have a look at this answer and this answer for some additional implementation examples.

    By using a Strategy Pattern, you decouple your application from the DI container and then it can be unit tested separately from the DI container.

    Option 3

    It is unclear whether the objects that you want to translate between will have dependencies. If so, using the factory you already came up with is a better fit than the Strategy pattern as long as you treat it as part of the composition root. This also means that the factory should be treated as an untestable class, and it should contain as little logic as necessary to complete its task.