1

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?

NightOwl888
  • 55,572
  • 24
  • 139
  • 212
Scott Simontis
  • 748
  • 1
  • 10
  • 24
  • 3
    This probably belongs on programmers but I will note that I don't see an issue with this. Your Unity-based factory is likely relatively trivial and is decoupled from any implementation of an `ITranslator` or its dependants. Just isolate the factory to a separate bootstrapping namespace and be done with it. Any way of avoiding this is going to be hugely over-engineered. This way works, is simple and elegant and has no real practical disadvantages. Don't overengineer just because this method is arbitrarily branded an "anti-pattern." – Ant P Mar 17 '15 at 19:22
  • 1
    It's unfortunate that Mark Seeman labelled Service Locator an anti-pattern. His article goes on to describe some of the disadvantages of a Service Locator, but that doesn't make it an anti-pattern per se. It is only an anti-pattern for those cases where the disadvantages are not an acceptable tradeoff for the benefits. – Robert Harvey Mar 17 '15 at 19:42
  • 2
    @AntP - I would call being tightly coupled to the DI container so it can't be unit tested easily a practical disadvantage. In 99% of all cases, you can engineer an application to work without a service locator using an existing pattern that doesn't require a lot of effort to engineer. – NightOwl888 Mar 17 '15 at 19:51
  • @NightOwl888 - Having a set of handlers that implement a generic interface, each specifying a different type argument, and having a factory provide those implementations in response to a specified runtime type is often extremely useful, as is leveraging an IoC container in the implementation of said factory. Most IoC containers will allow you to inject their registration contexts so you do not need to couple any interfaces to them and the context itself can still be mocked if you really feel a pressing need to unit test a factory that simply delegates a call to a third-party resolver. – Ant P Mar 17 '15 at 20:37
  • Nope. Service Locator is still an anti-pattern. However, Service Locator is a pattern to use within your application. If you access the container from WITHIN your Composition Root, this is NOT an implementation of the Service Locator anti-pattern. Do read [this](http://blog.ploeh.dk/2011/08/25/ServiceLocatorrolesvs.mechanics/). Long story short, as long as you define your Translator Factory *implementation* INSIDE your Composition Root, you are fine and are not applying the service locator pattern. – Steven Mar 17 '15 at 22:07

4 Answers4

2

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.

Community
  • 1
  • 1
NightOwl888
  • 55,572
  • 24
  • 139
  • 212
2

This doesn't really answer your larger question, but your search for a means of storing simple mapping functions without creating legions of trivial mapping classes* led to this nested map keyed by Source, then Destination types (I borrowed heavily from Darin's answer here):

public class TranslatorDictionary
{
    private readonly IDictionary<Type, IDictionary<Type, Delegate>> _mappings
        = new Dictionary<Type, IDictionary<Type, Delegate>>();

    public TDest Map<TSource, TDest>(TSource source)
    {
        IDictionary<Type, Delegate> typeMaps;
        Delegate theMapper;
        if (_mappings.TryGetValue(source.GetType(), out typeMaps) 
            && typeMaps.TryGetValue(typeof(TDest), out theMapper))
        {
            return (TDest)theMapper.DynamicInvoke(source);
        }
        throw new Exception(string.Format("No mapper registered from {0} to {1}", 
            typeof(TSource).FullName, typeof(TDest).FullName));
    }

    public void AddMap<TSource, TDest>(Func<TSource, TDest> newMap)
    {
        IDictionary<Type, Delegate> typeMaps;
        if (!_mappings.TryGetValue(typeof(TSource), out typeMaps))
        {
            typeMaps = new Dictionary<Type, Delegate>();
            _mappings.Add(typeof (TSource), typeMaps);
        }

        typeMaps[typeof(TDest)] = newMap;
    }
}

Which would then allow registration of mapping Funcs

// Bootstrapping
var translator = new TranslatorDictionary();
translator.AddMap<Foo, Bar>(
    foo => new Bar{Name = foo.Name, SurrogateId = foo.ID});
translator.AddMap<Bar, Foo>(bar => 
   new Foo { Name = bar.Name, ID = bar.SurrogateId, Date = DateTime.MinValue});

// Usage
var theBar = translator.Map<Foo, Bar>(new Foo{Name = "Foo1", ID = 1234, Date = DateTime.Now});
var theFoo = translator.Map<Bar, Foo>(new Bar { Name = "Bar1", SurrogateId = 9876});

Obviously, instead of reinventing wheels, one should instead opt for a more proven mapper like AutoMapper. With appropriate unit test coverage of each mapping, any concerns about fragility of the automagic mapping leading to regression issues can be avoided.

* C# can't instantiate anonymous classes which implement your ITranslator interface in .NET (unlike Java), so each ITranslator mapping will need to be a named class.

NightOwl888
  • 55,572
  • 24
  • 139
  • 212
StuartLC
  • 104,537
  • 17
  • 209
  • 285
  • 1
    I got sidetracked and forgot to reply, but +1 for some pretty crafty code. I had seen another coworker do something like this, but I could not really understand what was going on while reading through the code. Your example helped me wrap my head around this, thanks! – Scott Simontis Apr 09 '15 at 17:27
  • Now that we are embracing functions as first class variable citizens, this becomes quite a natural pattern after a while. But use AutoMapper - my solution isn't going to hold up with graphs etc :) – StuartLC Apr 09 '15 at 18:37
  • I just started using AutoMapper on this project as well, and it kicks ass! We are actually using it for most of the translation, but the ITranslator construct was already in place. It seems really awkward having to use this abstraction when 95% of the translators are just returning Mapper.Map(mySource)! I am really interested in functional programming, I have started trying to learn F# in my spare time. I have a lot to learn but I cna already tell it's going to be an enlightening experience! – Scott Simontis Apr 09 '15 at 18:41
1

I am afraid that you cannot.

By unit testing a piece of code, you need to know your input, and expected output. If it is so generic that you don't specify what it is, imagine how the compiler/unit test code know what it is expected?

NightOwl888
  • 55,572
  • 24
  • 139
  • 212
Bin Qian
  • 11
  • 1
0

First of all, Service locator is not an anti pattern. If we label patterns as anti-patterns just because they doesn't work for some use cases we would just have anti patterns left.

Regarding Unity you are taking the wrong approach. You do not unit test interfaces. You should unit test each class that implements that interface.

If you want to make sure that all implementations are correctly registered in the container you should create a test class that tries to resolve each implementation using the composition root in the real application.

If you just build another container for your unit tests you do not have any real proof that the actual application would work.

Summary:

  1. Unit test each converter
  2. Create a test that makes sure that all converters are registered in the real composition root.
NightOwl888
  • 55,572
  • 24
  • 139
  • 212
jgauffin
  • 99,844
  • 45
  • 235
  • 372
  • 1
    I know this is an old argument, but I still disagree. The Service Locator [IS an anti-pattern](http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/) for Line Of Business applications and even for reusable libraries it is questionable, because [there are](http://blog.ploeh.dk/2014/05/19/di-friendly-framework/) [better alternatives](http://blog.ploeh.dk/2014/05/19/di-friendly-library/). But for the rest, I agree with your answer :-) – Steven Mar 17 '15 at 22:00
  • However, do note that calling into the container from within your factory is NOT an implication of the Service Locator anti-pattern, simply because this factory should be part of your Composition Root, as Mark explains [here](http://blog.ploeh.dk/2011/08/25/ServiceLocatorrolesvs.mechanics/) clearly. – Steven Mar 17 '15 at 22:03
  • I've found one use case for ServiceLocators in the .Net world is for legacy tech stacks like WebForms or ASMX web services, where hooks for delegating root object instantiation via an IoC isn't available out of the box. But this is for top level only - for everything else, there is DI. – StuartLC Mar 18 '15 at 06:47
  • @Steven `The Service Locator IS an anti-pattern for Line Of Business applications`. I have never seen someone say `This is a anti-pattern, but just for XXX`. It's either a pattern (which always have specific use cases) or it's an anti-pattern (i.e. no use cases). You can't have both. – jgauffin Mar 20 '15 at 19:06
  • The very definition of Anti-pattern states just that: `An anti-pattern (or antipattern) is a common response to a recurring problem that is usually ineffective and risks being highly counterproductive`. – jgauffin Mar 20 '15 at 19:06