120

I most commonly am tempted to use "bastard injection" in a few cases. When I have a "proper" dependency-injection constructor:

public class ThingMaker {
    ...
    public ThingMaker(IThingSource source){
        _source = source;
    }

But then, for classes I am intending as public APIs (classes that other development teams will consume), I can never find a better option than to write a default "bastard" constructor with the most-likely needed dependency:

    public ThingMaker() : this(new DefaultThingSource()) {} 
    ...
}

The obvious drawback here is that this creates a static dependency on DefaultThingSource; ideally, there would be no such dependency, and the consumer would always inject whatever IThingSource they wanted. However, this is too hard to use; consumers want to new up a ThingMaker and get to work making Things, then months later inject something else when the need arises. This leaves just a few options in my opinion:

  1. Omit the bastard constructor; force the consumer of ThingMaker to understand IThingSource, understand how ThingMaker interacts with IThingSource, find or write a concrete class, and then inject an instance in their constructor call.
  2. Omit the bastard constructor and provide a separate factory, container, or other bootstrapping class/method; somehow make the consumer understand that they don't need to write their own IThingSource; force the consumer of ThingMaker to find and understand the factory or bootstrapper and use it.
  3. Keep the bastard constructor, enabling the consumer to "new up" an object and run with it, and coping with the optional static dependency on DefaultThingSource.

Boy, #3 sure seems attractive. Is there another, better option? #1 or #2 just don't seem worth it.

Patrick Szalapski
  • 8,738
  • 11
  • 67
  • 129
  • 1
    Why is the dependency on DefaultThingSource any worse than the dependency on IThingSource? Go with #3. – Fernando Jul 18 '11 at 14:09
  • 11
    Because DefaultThingSource is a potentially very specific implementation with a dependency on an external system in another assembly which could well be obsolete in a few months; IThingSource is in the will always be correct and never be tied to a specific implementation. – Patrick Szalapski Jul 18 '11 at 14:34
  • Obviously I do not know how complex DefaultThingSource is. However, I would wager that if you keep things simple and design it well, in reality you will find that your code will not be obsolete as quickly as you fear -- certainly not quickly enough to justify all this heartache. – Fernando Jul 18 '11 at 14:46
  • 2
    This is the problem with good dependency inversion - it may be more flexible and have better design, but it's not as easy for a new programmer to use. – Phil Jul 18 '11 at 17:09
  • You may also want to take a look at this article: http://lostechies.com/jimmybogard/2009/07/03/how-not-to-do-dependency-injection-in-nerddinner/ – JCallico Jul 18 '11 at 19:16
  • Also note that this practice is also known as "Poor Man’s Dependency Injection". – JCallico Jul 18 '11 at 19:30
  • @Patrick: "Because DefaultthingSource is ... obsolete in a few months" - Then it is a very bad candidate for a "default" in a public API either. I vote for #2 – chiccodoro Jul 19 '11 at 14:33
  • That's why I used "potentially" obsolete--we don't know how it will evolve in usage. We hope our consumers find all kinds of valuable ways to use our software. – Patrick Szalapski Jul 19 '11 at 14:43

12 Answers12

63

As far as I understand, this question relates to how to expose a loosely coupled API with some appropriate defaults. In this case, you may have a good Local Default, in which case the dependency can be regarded as optional. One way to deal with optional dependencies is to use Property Injection instead of Constructor Injection - in fact, this is sort of the poster scenario for Property Injection.

However, the real danger of Bastard Injection is when the default is a Foreign Default, because that would mean that the default constructor drags along an undesirable coupling to the assembly implementing the default. As I understand this question, however, the intended default would originate in the same assembly, in which case I don't see any particular danger.

In any case you might also consider a Facade as described in one of my earlier answers: Dependency Inject (DI) "friendly" library

BTW, the terminology used here is based on the pattern language from my book.

Community
  • 1
  • 1
Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • 3
    +1 The DI friendly library response is an absolute must read in this space and will enable you to think through this stuff from first principles. – Ruben Bartelink Jul 19 '11 at 11:57
  • 2
    Good thought about the difference between local defaults and foreign defaults. In this case, it is indeed a local default, albeit one that the consumer may well have a reason to override. In this case, providing two constructors hints at "use the default or inject your own" most simply. Using only one default constructor with property injection doesn't do enough to advertise the injectible dependency. Do you agree? – Patrick Szalapski Jul 20 '11 at 02:25
  • 4
    I agree that overloaded constructors are fine in this situation. I don't agree that Property Injection doesn't do enough to advertise that users can override the dependency, as it's a well-known pattern, but I'll grant that overloaded constructors are marginally more 'in your face' :) – Mark Seemann Jul 20 '11 at 07:21
  • 5
    Cannot recommend @MarkSeemann's book enough for all these issues of DI and IoC – Nick Hodges May 23 '13 at 15:20
  • @MarkSeemann - regarding the facade, I'm having trouble reconciling the idea that all I've done is move the bastard injection to another class. What is your opinion of declaring the default dependency in the config file and loading it via late binding? – zomf Jul 01 '14 at 18:01
  • 1
    @zomf Forcing configuration file settings on library users is not a particularly friendly approach. It locks down clients to use the configuration system, which is inflexible. What if you want to load config values from a database instead of a config file? What if you want to vary configuration values in unit tests? What if you want to make the configuration value configurable via the UI? ...FWIW, I've expanded on the DI friendly library recommendations [here](http://blog.ploeh.dk/2014/05/19/di-friendly-library), including more code examples. – Mark Seemann Jul 02 '14 at 08:01
33

My trade-off is a spin on @BrokenGlass:

1) Sole constructor is parameterized constructor

2) Use factory method to create a ThingMaker and pass in that default source.

public class ThingMaker {
  public ThingMaker(IThingSource source){
    _source = source;
  }

  public static ThingMaker CreateDefault() {
    return new ThingMaker(new DefaultThingSource());
  }
}

Obviously this doesn't eliminate your dependency, but it does make it clearer to me that this object has dependencies that a caller can deep dive into if they care to. You can make that factory method even more explicit if you like (CreateThingMakerWithDefaultThingSource) if that helps with understanding. I prefer this to overriding the IThingSource factory method since it continues to favor composition. You can also add a new factory method when the DefaultThingSource is obsoleted and have a clear way to find all the code using the DefaultThingSource and mark it to be upgraded.

You covered the possibilities in your question. Factory class elsewhere for convenience or some convenience within the class itself. The only other unattractive option would be reflection-based, hiding the dependency even further.

Steve Jackson
  • 1,330
  • 1
  • 15
  • 25
  • 3
    I still don't get why this is better than the default constructor--it has all the drawbacks with no added benefit; it just makes the dependency more obfuscated to anyone reading the code. – Patrick Szalapski Jul 18 '11 at 16:35
  • 18
    @Patrick In my opinion it makes the dependency more explicit than using the default constructor. The way the class is designed seems to indicate that the dependency is important (it's injected in the constructor) so I would make that explicit. The default constructor tells me: "Don't worry about this dependency I only created an extra constructor for testing or a one-off case". The factory method tells me "There is a dependency to be aware of, but here's the most likely implementation you're looking for" – Steve Jackson Jul 18 '11 at 16:42
  • 2
    Shouldn't this be "CreateDefault" with a capital C? – Hinek Jul 19 '11 at 07:37
  • 2
    This solution doesn't make clear how this class is intended to be used. The first thing you look at, when using a class for the first time, is the constructors, if I see only one constructor I will assume that I MUST provide an instance of IThingSource in order to use ThingMaker. Chances are that if I were a user of such a class I would never had discovered the CreateDefault method. – JCallico Jul 19 '11 at 14:08
  • I am still not convinced, Steve. While the number of upvotes might indicate I am wrong, I do not think that having a default constructor means "I only created an extra constructor for testing or a one-off case." Instead, I think it means, "Use the default or inject your own." You also said that you think the default constructor means "Don't worry about this dependency"--and that I agree with. They can use the default constructor if they don't want to worry about the other. – Patrick Szalapski Jul 20 '11 at 02:14
  • 1
    @Patrick - Fair enough. This is merely an alternative if you want to emphasize the dependency a bit more. There's always a trade-off in these decisions and it's really a question of where you want to put your emphasis. If the DefaultThingSource is the primary option for most callers then I would encourage using it in the default constructor. If you want the caller to consider supplying their own IThingSource then I think this is an acceptable middle ground. If you want to force the user to supply an IThingSource then I start leaning towards factory classes. – Steve Jackson Jul 20 '11 at 04:13
8

One alternative is to have a factory method CreateThingSource() in your ThingMaker class that creates the dependency for you.

For testing or if you do need another type of IThingSource you would then have to create a subclass of ThingMaker and override CreateThingSource() to return the concrete type you want. Obviously this approach only is worth it if you mainly need to be able to inject the dependency in for testing, but for most/all other purposes do not need another IThingSource

BrokenGlass
  • 158,293
  • 28
  • 286
  • 335
  • 4
    I thought of this, but it has all the drawbacks of the bastard constructor option with none of the benefits! Right? – Patrick Szalapski Jul 18 '11 at 13:48
  • 1
    Agreed for the most part - I do use the default constructor to create the "default" dependencies myself and see nothing wrong with it. This is different though because it would allow you not having to have the dependency in *any* constructor - this way you would avoid another what you call it "bastard constructor", making the public interface easier to parse. This does only make sense though if changing the dependency is only relevant for testability of the class. – BrokenGlass Jul 18 '11 at 14:57
6

I vote for #3. You'll be making your life--and the lives of other developers--easier.

Fernando
  • 1,239
  • 1
  • 16
  • 27
  • 6
    Can you explain *how*? What problems does the third approach avoid? – Adam Lear Jul 18 '11 at 13:49
  • 1
    #1 requires extra work for the consumers. #2 requires extra work on both the consumers and Patrick. Neither #1 nor #2 provide enough benefits to outweigh the perceived disadvantage of having a dependency on DefaultThingSource. – Fernando Jul 18 '11 at 14:03
  • I think that @Mark Seemann's answer comes closest to giving good design while living in the real world, but this answer is simple and good too. :) – Patrick Szalapski Jul 20 '11 at 20:52
6

If you have to have a "default" dependency, also known as Poor Man’s Dependency Injection, then you have to initialize and "wire" the dependency somewhere.

I will keep the two constructors but have a factory just for the initialization.

public class ThingMaker
{
    private IThingSource _source;

    public ThingMaker(IThingSource source)
    {
        _source = source;
    }

    public ThingMaker() : this(ThingFactory.Current.CreateThingSource())
    {
    }
}

Now in the factory create the default instance and allow the method to be overrided:

public class ThingFactory
{
    public virtual IThingSource CreateThingSource()
    {
        return new DefaultThingSource();
    }
}

Update:

Why using two constructors: Two constructors clearly show how the class is intended to be used. The parameter-less constructor states: just create an instance and the class will perform all of it's responsibilities. Now the second constructor states that the class depends of IThingSource and provides a way of using an implementation different than the default one.

Why using a factory: 1- Discipline: Creating new instances shouldn't be part of the responsibilities of this class, a factory class is more appropriate. 2- DRY: Imagine that in the same API other classes also depend on IThingSource and do the same. Override once the factory method returning IThingSource and all the classes in your API automatically start using the new instance.

I don't see a problem in coupling ThingMaker to a default implementation of IThingSource as long as this implementation makes sense to the API as a whole and also you provide ways to override this dependency for testing and extension purposes.

JCallico
  • 1,446
  • 18
  • 25
  • 1
    Why have a factory at all, then? Just put the logic in the ThingMaker default constructor. – Patrick Szalapski Jul 19 '11 at 03:00
  • This is __necessary__ if you absolutely have to have a public ctor. If you're not in that boat, this is a bad idea. Can you make that clear in your answer or I'd feel it should be downvoted on the basis that it doesnt add anything the other answer doesnt already do and is a significantly worse implementation given that it doesnt remove the coupling issue. – Ruben Bartelink Jul 19 '11 at 11:54
  • @Ruben Bartelink: I've updated my answer explaining the design choices. – JCallico Jul 19 '11 at 13:50
  • "Creating new instances [of itself] shouldn't be a part of the responsibility of this class." Really? Couldn't you say that about every class every created? I think factory methods/classes have their place but I can't just always use them for everything. – Patrick Szalapski Jul 20 '11 at 02:18
  • @Patrick: It's a matter of preference. I personally try to isolate the creation of new instances of meaningful classes to only one place in my APIs hence the factory usage. Also by using a factory the default IThingSource can be easily modified from a central place. As I said before personal preference. – JCallico Jul 20 '11 at 13:36
  • @JCallico: To make it clear, your edits have not swayed me. I missed a key word in my original comment -- If you absolutely need a public __parameterless__ constructor (for Serialization or because its an Attribute) then fine. In all other cases, the bastard injector is completely bad news. If you make that clear/agree, I upvote, otherwise +0/meh/your answer just looks at a glance as if it was advocating bad practices but I wouldnt downvote becuase ther's one important technique in it. This is about removing redundant static references to the max degree possible. – Ruben Bartelink Jul 20 '11 at 15:44
  • @Ruben Bartelink: In order for callers to instantiate the ThingMaker class and use the default dependency for IThingSource, which is a functionality that the author of the question wants, you need a parameterless constructor, is either that or any other of the less clear solutions suggested like the static CreateDefault method which I don't consider adequate. Thanks for all your feedback. – JCallico Jul 20 '11 at 19:35
  • @JCallico: OK. I've re-read the question and agree that this is a clean impl of #3. I dont agree with #3 except ion my cited cases (Attributes and Serialization where a pub paramless ctor is reqd) so this wouldnt ordinarily get an upvote from me but the debate and the fact you're adding something a touch different deserves an upvote. – Ruben Bartelink Jul 21 '11 at 08:29
2

You are unhappy with the OO impurity of this dependency, but you don't really say what trouble it ultimately causes.

  • Is ThingMaker using DefaultThingSource in any way that does not conform to IThingSource? No.
  • Could there come a time where you would be forced to retire the parameterless constructor? Since you are able to provide a default implementation at this time, unlikely.

I think the biggest problem here is the choice of name, not whether to use the technique.

Amy B
  • 108,202
  • 21
  • 135
  • 185
  • I do not like the static dependency on DefaultThingSource. – Patrick Szalapski Jul 18 '11 at 13:54
  • Why do you not like the dependency? – Amy B Jul 18 '11 at 13:58
  • I agree with you, David. Except that I would not use the phrase "OO impurity" because there is nothing impure about the dependency. It is simply a judgement call as to have it or not. I agree that having it is not a problem. – Fernando Jul 18 '11 at 14:07
  • @David B: I would suppose it isn't liked because once a user starts using AwesomeNewThing, it will still have an obsolete unused dependency on DefaultThingSource. – Zan Lynx Jul 18 '11 at 20:06
  • That dependency isn't public knowledge and might be changed without disrupting the user. – Amy B Jul 18 '11 at 21:01
2

The examples usually related to this style of injection are often extremely simplisitic: "in the default constructor for class B, call an overloaded constructor with new A() and be on your way!"

The reality is that dependencies are often extremely complex to construct. For example, what if B needs a non-class dependency like a database connection or application setting? You then tie class B to the System.Configuration namespace, increasing its complexity and coupling while lowering its coherence, all to encode details which could simply be externalized by omitting the default constructor.

This style of injection communicates to the reader that you have recognized the benefits of decoupled design but are unwilling to commit to it. We all know that when someone sees that juicy, easy, low-friction default constructor, they are going to call it no matter how rigid it makes their program from that point on. They can't understand the structure of their program without reading the source code for that default constructor, which isn't an option when you just distribute the assemblies. You can document the conventions of connection string name and app settings key, but at that point the code doesn't stand on its own and you put the onus on the developer to hunt down the right incantation.

Optimizing code so those who write it can get by without understanding what they are saying is a siren song, an anti-pattern that ultimately leads to more time lost in unraveling the magic than time saved in initial effort. Either decouple or don't; keeping a foot in each pattern diminishes the focus of both.

Bryan Watts
  • 44,911
  • 16
  • 83
  • 88
  • Sorry, you haven't convinced me. Two constructors--one makes the easy things easy, the other makes the difficult things possible. What you want me to do seems to make the easy things difficult. – Patrick Szalapski Jul 20 '11 at 20:45
  • @Patrick Szalapski: My point is 1) the consumers of your code can't understand the structure of their program without source code or documentation, 2) not every dependency's constructor is simple, and 3) your objects take on more knowledge and responsibility, diluting their focus. If those points don't convince you, nothing ever will. Build an entire system in that style and evaluate for yourself whether it makes things easier. – Bryan Watts Jul 20 '11 at 21:47
1

For what it is worth, all the standard code I've seen in Java does it like this:

public class ThingMaker  {
    private IThingSource  iThingSource;

    public ThingMaker()  {
        iThingSource = createIThingSource();
    }
    public virtual IThingSource createIThingSource()  {
        return new DefaultThingSource();
    }
}

Anybody who doesn't want a DefaultThingSource object can override createIThingSource. (If possible, the call to createIThingSource would be somewhere other than the constructor.) C# does not encourage overriding like Java does, and it might not be as obvious as it would be in Java that the users can and perhaps should provide their own IThingSource implementation. (Nor as obvious how to provide it.) My guess is that #3 is the way to go, but I thought I would mention this.

RalphChapin
  • 3,108
  • 16
  • 18
  • 1
    +0:-.5: Shouldnt call virtuals in a ctor (100% sure for OO in general, 99% sure for Java). -.5: Doesnt address the coupling concern +1: An interesting variation on the technique which may help people think, even if youi wouldnt use it as an actual solution – Ruben Bartelink Jul 19 '11 at 11:56
  • 1
    @Ruben Bartelink: yes, virtual calls in constructors should be avoided: http://stackoverflow.com/questions/119506/virtual-member-call-in-a-constructor – JCallico Jul 19 '11 at 16:12
  • I put the virtual call in the constructor to keep the example as simple as possible. However, in this case it was me that put it there. The "standard code I've seen in Java" *does not* make the call to the createIThingSource() equivalent in the constructor! – RalphChapin Jul 19 '11 at 20:34
0

Just an idea - perhaps a bit more elegant but sadly doesn't get rid of the dependency:

  • remove the "bastard constructor"
  • in the standard constructor you make the source param default to null
  • then you check for source being null and if this is the case you assign it "new DefaultThingSource()" otherweise whatever the consumer injects
Yahia
  • 69,653
  • 9
  • 115
  • 144
0

Have an internal factory (internal to your library) that maps the DefaultThingSource to IThingSource, which is called from the default constructor.

This allows you to "new up" the ThingMaker class without parameters or any knowledge of IThingSource and without a direct dependency on DefaultThingSource.

Jonathan van de Veen
  • 1,016
  • 13
  • 27
  • This sounds like option #3, but just split into multiple classes. It doesn't get rid of the static dependency. – Patrick Szalapski Jul 18 '11 at 13:53
  • ThingMaker is no longer staticly dependent on DefaultThingSource as it uses the factory. If you now have a new implementation of IThingSource, you simply change the mapping in only one place, your factory. – Jonathan van de Veen Jul 25 '11 at 11:58
  • Yes, but ThingMaker has a static dependency on the factory, and the factory has a static dependency on our default Thing source. We want to get rid of the static dependency altogether. – Patrick Szalapski Aug 24 '11 at 16:58
  • Ah, so this is a theoretical excercise then? Because I can't see the practical advantages to having no static dependencies at all in your case. – Jonathan van de Veen Aug 29 '11 at 05:34
0

For truly public APIs, I generally handle this using a two-part approach:

  1. Create a helper within the API to allow an API consumer to register "default" interface implementations from the API with their IoC container of choice.
  2. If it is desirable to allow the API consumer to use the API without their own IoC container, host an optional container within the API that is populated the same "default" implementations.

The really tricky part here is deciding when to activate the container #2, and the best choice approach will depend heavily on your intended API consumers.

Nicole Calinoiu
  • 20,843
  • 2
  • 44
  • 49
0

I support option #1, with one extension: make DefaultThingSource a public class. Your wording above implies that DefaultThingSource will be hidden from public consumers of the API, but as I understand your situation there's no reason not to expose the default. Furthermore, you can easily document the fact that outside of special circumstances, a new DefaultThingSource() can always be passed to the ThingMaker.

JSBձոգչ
  • 40,684
  • 18
  • 101
  • 169