How to turn this Service Locator pattern into true Dependency Injection pattern?

Question

I asked a more general question a minute ago: How to organize DI Framework usage in an application?, and the feedback I got was that I was using a Service Locator Pattern rather than true DI as is pointed out by Martin Fowler here: http://martinfowler.com/articles/injection.html

Actually, I read that article just the other day, but apparently haven't quite grasped it.

So let's say I have the following code:

interface ICardReader
{
    string GetInfo();
    void SetDebugMode(bool value);
    void Initialize(string accountToken);
    void ShowAmount(string amount);
    void Close();

    ICreditCardInfo GetCardInfo();
}

public class MagTekIPAD: ICardReader
{
    public ICreditCardInfo GetCardInfo()
    {
        var card = GetCardDataFromDevice();

        // apparently the following line is wrong?
        var ccInfo = Inject<ICreditCardInfo>.New(); 

        ccInfo.Track1 = MakeHex(card.EncTrack1);
        ccInfo.Track2 = MakeHex(card.EncTrack2);
        ccInfo.MagSignature = MakeHex(card.EncMP);
        ccInfo.MagSwipeKeySN = MakeHex(card.KSN);
        ccInfo.MagSignatureStatus = MakeHex(card.MPSts);
        ccInfo.MagDeviceSN = ipad.Serial;
        ccInfo.MSREncryptType = "MAGENSA_V5";

        return ccInfo;
    }

    // Other implementation details here ...
}

In this example I could inject the dependency into the constructor—and I think that's the correct way to fix 'this' scenario.

But what if I actually need to create an unknown number of the object in question (or are there any other legitimate reason I'd have a need to create the dependency on the fly in the class)?

@typeoneerror I noticed some edits were done to the code block, but now there's no color. Not a big deal but just wondering why that is? — Brandon Moore, Feb 21 '12 at 06:33
unsure. could be the missing bracket at the end (added). I just added http://stackoverflow.com/editing-help#syntax-highlighting — typeoneerror, Feb 21 '12 at 06:45
It looks as though you are attempting to create an Entity or Value object with DI, which is typically not the right approach: http://stackoverflow.com/questions/4835046/why-not-use-an-ioc-container-to-resolve-dependencies-for-entities-business-objec/4836790#4836790 — Mark Seemann, Feb 22 '12 at 01:21
Still, if you must create an arbitrary number of instances of a dependency, you can use an Abstract Factory: http://stackoverflow.com/questions/1943576/is-there-a-pattern-for-initializing-objects-created-via-a-di-container/1945023#1945023 — Mark Seemann, Feb 22 '12 at 01:22

score 7 · Accepted Answer · answered Feb 21 '12 at 16:56

7

This example gives me the impression that you try to create a data transfer object namingly ICreditCardInfo using an IoC container. Such objects should not have any real dependencies like a service. The proper way to create DTOs is to use the new operator:

return new CreditCardInfo(
        MakeHex(card.EncTrack1),
        MakeHex(card.EncTrack2),
        MakeHex(card.EncMP),
        MakeHex(card.KSN),
        MakeHex(card.MPSts),
        ipad.Serial,
        "MAGENSA_V5");

answered Feb 21 '12 at 16:56

Remo Gloor

32,665
4
68
98

Thanks. I assumed that it was best to limit dependencies and if I create a new CreditCardInfo here then I have to take a dependency on the assembly that contains that class. So two questions: 1) Are you suggesting that should be perfectly okay? 2) If there were methods in CreditCardInfo that did stuff then would that change your answer? – Brandon Moore Feb 22 '12 at 08:54
1

@BrandonMoore 1) If there is no reason for having the CreditCardInfo in a different assembly then I wouldn't do this. Normally, the interface is shared between assemblies and not the implementation. 2) CreditCardInfo seems for me to be a proper DTO. Therefor I wouldn't add functionallity there but rather have some service that provides the functionallity and take a ICreditCardInfo as input. But I don't know your system. If there is a good reason to add the functionallity there and it requires real dependencies then yes - in this case you would have to create it using the IoC container – Remo Gloor Feb 22 '12 at 13:53
Thanks, I really appreciate the fact that you both provided me with info on how you believe it ought to work without neglecting to directly answer the question asked as well. I wish everyone could learn to answer questions this way as both sides of the answer are equally valuable to me. – Brandon Moore Feb 23 '12 at 00:06

Sebastian Weber · Answer 2 · 2012-02-21T06:48:55.797

3

Inject a factory for ICreditCardInfo objects into the constructor of MagTekIPAD

public class MagTekIPAD : ICardReader
{
  private readonly Func<ICreditCardInfo> factory;
  public MagTekIPAD(Func<ICreditCardInfo> factory)
  {
    this.factory = factory;
  }
  public ICreditCardInfo GetCardInfo()
  {
    var info = factory();
    // ...
    return info;
  }
}

Several containers can auto-generate Func<T> delegates if they know how to create instances of T so you don't have to define factory interfaces or abstract factory classes.

edited Feb 21 '12 at 06:48

answered Feb 21 '12 at 06:36

Sebastian Weber

6,766
2
30
49

Thanks... still wrapping my brain around how exactly to do that but everything's already making a lot more sense so I figure in 24 hours I'll be wondering what was so hard about all this :) – Brandon Moore Feb 21 '12 at 07:07
Although I don't like the use ofFunc delegates as factories, I completely agree with the general advice of using factories. – Steven Feb 21 '12 at 12:11
@Steven If you prefer more explicit factory definitions but don't want to implement them there are the [Castle Windsor Typed Factory Facilities](http://docs.castleproject.org/Windsor.Typed-Factory-Facility-interface-based-factories.ashx) and its [port to Microsoft Unity](http://tecx.codeplex.com/wikipage?title=Auto-generated%20Factories%20with%20Unity&referringTitle=Home). Define the factory interface and tell the container to do the rest. – Sebastian Weber Feb 21 '12 at 12:55
@SebastianWeber: I rather define everything by hand :-). Nothing is more clear and clean than just a simple `ICreditCardInfoFactory`. – Steven Feb 21 '12 at 13:10
1

@Steven I don't say that delegates are preferable to explicit factory definitions. I just would not bother to implement those interfaces if my infrastructure can take care of that for me. – Sebastian Weber Feb 21 '12 at 13:26
@Sebastian: It's a matter of tast. I had the discussion about Func before here at SO. Not everybody agrees with me, which is fine :-) – Steven Feb 21 '12 at 14:52

score -1 · Answer 3 · answered Feb 21 '12 at 06:35

-1

As Fowler pointed out, Service Locator is the more direct approach and is less error prone.

Some Dependency Injection frameworks require you to declare whether you are dealing with Singleton or they can have different lifetimes.

answered Feb 21 '12 at 06:35

Chui Tey

5,436
2
35
44

8

-1 ServiceLocator is [considered an anti-pattern in modern application architectures](http://blog.ploeh.dk/2010/02/03/ServiceLocatorIsAnAntiPattern.aspx). It hides dependencies and thus is not less but more error prone. – Sebastian Weber Feb 21 '12 at 06:39
1

I gave you a +1 to offset the naysayers just cause I'm not entirely convinced that Service Locator is such a horrible thing. I think it's easier to develop with that pattern and often people will forgo that benefit too easily for other perceived benefits or benefits that never quite materialize as hoped for. – Brandon Moore Feb 21 '12 at 07:08
Rather, I realize it's not the best pattern for certain scenarios but I think for many other scenarios DI wouldn't be signifacantly better than Service Locator in the long run. – Brandon Moore Feb 21 '12 at 07:10
Thanks. There is a certain level of complexity that comes with each layer of abstraction. I've worked with code where the intention has been obscured through excessive abstraction. DI is no better when setter-injection is involved. Some implementations use annotations/attributes on classes to declare these dependencies. These give some measure of confidence that the dependencies have been wired up correctly. – Chui Tey Feb 21 '12 at 08:39
1

Property injection should be reserved for "nice to have" dependencies. If your code won't work without a dependency, constructor injection should be the way to go. You can get an overview of what you need to run the code. No hidden surprises that make will you fail later. And the use of [DI attributes is another approach that is strongly disencouraged](http://blog.ploeh.dk/2010/01/27/WhatsSoDangerousAboutADIAttribute.aspx). – Sebastian Weber Feb 21 '12 at 09:15
Thanks Sebastian for the links. I think you've convinced me on the merits of constructor injection. On the whole, constructor injection enjoys a lot of clarity, and can be used sans framework, which is always a plus. I was concerned about how brittle code can be every time we added a "nice-to-have" dependency, but that can be done as a property DI. – Chui Tey Feb 21 '12 at 12:08

How to turn this Service Locator pattern into true Dependency Injection pattern?

3 Answers3

Linked