To “new” or not to “new”…

September 30th, 2008 · 24 Comments ·

by Miško Hevery

Dependency injection asks us to separate the new operators from the application logic. This separation forces your code to have factories which are responsible for wiring your application together. However, better than writing factories,  we want to use automatic dependency injection such as GUICE to do the wiring for us. But can DI really save us from all of the new operators?

Lets look at two extremes. Say you have a class MusicPlayer which needs to get a hold of AudioDevice. Here we want to use DI and ask for the AudioDevice in the constructor of the MusicPlayer. This will allow us to inject a test friendly AudioDevice which we can use to assert that correct sound is coming out of our MusicPlayer. If we were to use the new operator to instantiate the BuiltInSpeakerAudioDevice we would have hard time testing. So lets call objects such as AudioDevice or MusicPlayer “Injectables.” Injectables are objects which you will ask for in the constructors and expect the DI framework to supply.

Now, lets look at the other extreme. Suppose you have primitive “int” but you want to auto-box it into an “Integer” the simplest thing is to call new Integer(5) and we are done. But if DI is the new “new” why are we calling the new in-line? Will this hurt our testing? Turns out that DI frameworks can’t really give you the Integer you are looking for since it does not know which Integer you are referring to. This is a bit of a toy example so lets look at something more complex.

Lets say the user entered the email address into the log-in box and you need to call new Email(“a@b.com”). Is that OK, or should we ask for the Email in our constructor. Again, the DI framework has no way of supplying you with the Email since it first needs to get a hold of a String where the email is. And there are a lot of Strings to chose from. As you can see there are a lot of objects out there which DI framework will never be able to supply. Lets call these “Newables” since you will be forced to call new on them manually.

First, lets lay down some ground rules. An Injectable class can ask for other Injectables in its constructor. (Sometimes I refer to Injectables as Service Objects, but that term is overloaded.) Injectables tend to have interfaces since chances are we may have to replace them with an implementation friendly to testing. However, Injectable can never ask for a non-Injectable (Newable) in its constructor. This is because DI framework does not know how to produce a Newable. Here are some examples of classes I would expect to get from my DI framework: CreditCardProcessor, MusicPlayer, MailSender, OfflineQueue. Similarly Newables can ask for other Newables in their constructor, but not for Injectables (Sometimes I refer to Newables as Value Object, but again, the term is overloaded). Some examples of Newables are: Email, MailMessage, User, CreditCard, Song. If you keep this distinctions your code will be easy to test and work with. If you break this rule your code will be hard to test.

Lets look at an example of a MusicPlayer and a Song

class Song {
  Song(String name, byte[] content);
}
class MusicPlayer {
  @Injectable
  MusicPlayer(AudioDevice device);
  play(Song song);
}

Notice that Song only asks for objects which are Newables. This makes it very easy to construct a Song in a test. Music player is fully Injectable, and so is its argument the AudioDevice, therefore, it can be gotten from DI framework.

Now lets see what happens if the MusicPlayer breaks the rule and asks for Newable in its constructor.

class Song {
  String name;
  byte[] content;
  Song(String name, byte[] content);
}
class MusicPlayer {
  AudioDevice device;
  Song song;
  @Injectable
  MusicPlayer(AudioDevice device, Song song);
  play();
}

Here the Song is still Newable and it is easy to construct in your test or in your code. The MusicPlayer is the problem. If you ask DI framework for MusicPlayer it will fail, since the DI framework will not know which Song you are referring to. Most people new to DI frameworks rarely make this mistake since it is so easy to see: your code will not run.

Now lets see what happens if the Song breaks the rule and ask for Injectable in its constructor.

class MusicPlayer {
  AudioDevice device;
  @Injectable
  MusicPlayer(AudioDevice device);
}
class Song {
  String name;
  byte[] content;
  MusicPlayer palyer;
  Song(String name, byte[] content, MusicPlayer player);
  play();
}
class SongReader {
  MusicPlayer player
  @Injectable
  SongReader(MusicPlayer player) {
    this.player = player;
  }
  Song read(File file) {
    return new Song(file.getName(),
                    readBytes(file),
                    player);
  }
}

At first the world looks OK. But think about how the Songs will get created. Presumably the songs are stored on a disk and so we will need a SongReader. The SongReader will have to ask for MusicPlayer so that when it calls the new on a Song it can satisfy the dependencies of Song on MusicPlayer. See anything wrong here? Why in the world does SongReader need to know about the MusicPlayer. This is a violation of Law of Demeter. The SongReader does not need to know about MusicPlayer. You can tell since SongReader does not call any method on the MusicPlayer. It only knows about the MusicPlayer because the Song has violated the Newable/Injectable separation. The SongReader pays the price for a mistake in Song. Since the place where the mistake is made and where the pain is felt are not the same this mistake is very subtle and hard to diagnose. It also means that a lot of people make this mistake.

Now from the testing point of view this is a real pain. Suppose you have a SongWriter and you want to verify that it correctly serializes the Song to disk. Why do you have to create a MockMusicPlayer so that you can pass it into a Song so that you can pass it into the SongWritter. Why is MusicPlayer in the picture? Lets look at it from a different angle. Song is something you may want to serialize, and simplest way to do that is to use Java serialization. This will serialize not only the Song but also the MusicPlayer and the AudioDevice. Neither MusicPlayer nor the AudioDevice need to be serialized. As you can see a subtle change makes a whole lot of difference in the easy of testability.

As you can see the code is easiest to work with if we keep these two kinds objects distinct. If you mix them your code will be hard to test.  Newables are objects which are at the end of your application object graph. Newables may depend on other Newables as in CreditCard may depend on Address which may depend on a City but these things are leafs of the application graph. Since they are leafs, and they don’t talk to any external services (external services are Injectables) there is no need to mock them. Nothing behaves more like a String like than a String. Why would I mock User if I can just new User, Why mock any of these: Email, MailMessage, User, CreditCard, Song? Just call new and be done with it.

Now here is something very subtle. It is OK for Newable to know about Injectable. What is not OK is for the Newable to have a field reference to Injectable. In other words it is OK for Song to know about MusicPlayer. For example it is OK for an Injectable MusicPlayer to be passed in through the stack to a Newable Song. This is because the stack passing is independent of DI framework. As in this example:

class Song {
  Song(String name, byte[] content);
  boolean isPlayable(MusicPlayer player);
}

The problem becomes when the Song has a field reference to MusicPlayer. Field references are set through the constructor which will force a Law of Demeter violation for the caller and we will have hard time to test.

Tags: Advice · Testability

24 responses so far ↓

  • André Faria Gomes // Oct 1, 2008 at 10:29 am

    Very nice post. Enlightfull and easy to understand.

  • Graeme Foster // Oct 27, 2008 at 5:17 am

    Hi Misko,

    Some great info here – thanks. Can I ask a question though – how would you handle a scenario when you have a class which needs something that can be injected as-well as some contextual information.

    Lets say I’m looking at a list of customers and I want to select one to edit their details. I need to create a Customer View which might need a reference to a singleton (small s) service that might do things like update the customer in the db, but I also need a customer to supply context to the view.

    It feels to me like the View can’t operate without all of the above, but whilst I could use Dependency Injection to create 90% of it, how do I get the customer in? Or am I thinking about this in the wrong way!

    Thanks,

    Graeme

  • misko // Oct 27, 2008 at 12:20 pm

    @Graeme

    You bring about a very good point. For situations such as these you need to have some kind of nice framework to inject the Customer. I believe GUICE in 2.0 will have a concept of a converter where you can specify @RequestParameter(“custID”) Customer c and the GUICE will look up RequestParameter “custID” which is a String and than find a Converter which knows how to change a String -> Customer.

    the other way to solve this is to have a View ask for the customer in the execute method. So GUICE can satisfy constructor 100% and all of the others are passed in to the View through the stack as in execute(Customer c). This makes it a problem of the caller to get a hold of the customer. The caller will most likely be a servlet. which can fetch/create the customer.

    The key here is that the thing we want to test (The view) is fully injectable hence testable.

  • Graeme Foster // Oct 27, 2008 at 4:54 pm

    Out of those two approaches I definitely prefer the 2nd one.

    I was writing aMicrosoft CAB application (Smart Client) and in my case already had the Customer object.
    What I found myself doing before I slapped my wrists was declaring an additional constructor parameter for the Customer along with the injected services, dropping the Customer into the IoC container, and having it resolve the Customer a second later.

    It was a bit like black magic – it disappeared, then re-appeared!

    The 2nd approach is far more explicit. You create the View with the injected interfaces it requires, then you ask it to Execute with the Customer. I like that.

    Further to that it because there is no constructor logic happening it should be relatively easy to re-use the View if your application requires.

    Thanks,

    graeme

  • Graeme Foster // Oct 27, 2008 at 4:59 pm

    Sorry just to follow from this –

    With the first approach in a Smart Client world if you can reach the container to drop the Customer object in, then you’re in a real loosey goosey testing area… The container is either a Singleton (big S) in your local context or was passed through your application layers which will make things very difficult to test.

    If a method accepts a Container parameter in its constructor then it’s no better than accepting a Dictionary in-terms of testing. You have no idea what it wants from it!

    Everything seems to be coming back to your point of constructing your objects up-front, and then explicitly passing them where they are needed in your application.

    Thanks – I’m enjoying the thread!

    Graeme

  • Don Vince // Dec 30, 2008 at 7:00 am

    Great article, I’ve been trying to wrap my head around the subtleties.For a Logger class, an Injectable (a good sell on that can be found here: http://misko.hevery.com/code-reviewers-guide/flaw-brittle-global-state-singletons/ ) I can see that all/most the Injectable’s will have the Logger as a constructor parameter and will be merrily logging away to the object.What I’m not sure about is when a newable wants to do it’s logging.I think I could:Put the logger on the signature every method that does some logging (feels ugly)Ensure the newable’s have a fantastic ToString() method and let the Injectable’s do all the logging (feels nicer)Any further suggestions out there?

  • Design for Testability and “Domain-Driven Design” // Mar 16, 2009 at 10:35 pm

    [...] I have a question about your distinction between “newable”, value classes and “injectable” or “service” classes. [...]

  • Marty // Sep 10, 2009 at 12:04 am

    I’m still not really can follow this :(

    If your requirements now say that MusicPlayer has to have a name (String field) – it becomes a Newable and its not injectable anymore ?
    So what to do with the current MusicPlayer, because it also has the AudioAdvice field – and the injection fails…

    what is the solution for that ?

  • All About Google » How to think about OO // Oct 5, 2009 at 11:42 pm

    [...] Should User have a field reference to Ldap? The answer is no, because you may want to serialize the user to database but you don’t want to serialize the Ldap. See here. [...]

  • Making Zend_Auth more “Object Oriented” | Aviblock.com // Oct 29, 2009 at 6:13 pm

    [...] the Zend_Auth_Adapter_Interface. It seems to me that it violates one of Misko’s rules that injectables can depend on injectables, and newables can depend on newables, but the twain shall not …. So if my understanding of that rule is correct, $adapter->setIdentity($thisvarisanewable), would [...]

  • Sam // Nov 30, 2009 at 2:49 pm

    Hi Misko,
    I have a simple question. Because an injectable can never have a newable as a constructor parameter, at some point there must be an injectable implementation which has a parameterless constructor, right?

  • miw // Dec 4, 2009 at 3:29 pm

    Hi,

    @Sam

    Good point Sam, I also came to this conclusion and would like to hear about it.

  • Koen // Dec 17, 2009 at 10:24 am

    A database connection is an injectable. Yet the user credentials are newables. Unless you call db->connect(credentials). But that’s not really the job of the dependency injection container. So it seems like there are case in which the theory doesn’t hold.

  • Juraj Blahunka // Feb 6, 2010 at 5:15 pm

    @Misko What about injectables, that are required for object instantiation but can be changed with another values? (like your MusicPlayer, which needs AudioDevice)… Sure AudioDevice is needed in MusicPlayer’s constructor, but AudioDevice can change (from Speakers to Headset).. Should a SETTER be introduced? Doesn’t this bring ambiguity to the MusicPlayer class?

  • misko // Feb 7, 2010 at 7:56 pm

    @ Juraj,

    I would solve this in this way

    interface Output;
    class Speaker implements Output;
    class Headset implements Output;
    class OutputSelector(Speaker, Headset) implements Output;
    class MusicPlayer(Output);

    s = new Speaker()
    h = new Headset()
    os = new OutputSerector(s, h);
    mp = new MusicPlayer(os);

    os.seletOutput(…);
    wp.play();

  • Steven // Mar 25, 2011 at 7:00 am

    RE: Applying the ‘CleanCodeTalks’ guidelines to C++ code.

    I’m a big fan of the CleanCodeTalks presentations and follow their guidelines when I’m writing Java code.

    I’d like to apply these same guidelines when writing C++ code.

    In particular I’m looking to understand the best way to follow the dependency-injection and testability guidelines in order to:

    a. structure code to have object construction kept inside factories,

    b. keep calls to ‘new’ out of constructors,

    c. make it obvious to any user of a class
    what the dependencies of the class are
    just by the user looking at the parameter list
    of the constructor.

    My question is what is the best way to design a C++ class which has a field that has a “complex-type”, as the constructor for my class will have to invoke the constructor for the field.

    I’ve reviewed the guidelines and here’s what I’m thinking.
    To follow these guidelines I should:

    1. replace fields that have “complex-type” with a “pointer-to-complex-type”

    2. in the constructor declare a parameter of type “pointer-to-complex-type”.

    3. in the factory use ‘new’ to construct an object of type “pointer-to-complex-type”
    and pass that to the constructor of “my-class”.

    [ IIRC, I think the guidelines recommended that if the field had a "complex-type" that was a collection type - like std::vector - then there's no need to construct it externally from "my-class" constructor. ]

    I assume that as a general rule it wouldn’t be good design to keep the field as a “complex-type” and have the constructor declare a parameter of “ref-to-complex-type” as the copy-constructor for the “complex-type” is likely to be too expensive or possibly prohibited by the “complex-type” declaring it private.

    As an example, for a ‘MyFileLogger’ class that needs to access a ‘std::ofstream’ object, if I follow the guidelines (a, b, c) then the ‘MyFileLogger’ class that I might have written as:

    class MyFileLogger
    {
    std::ofstream theFileStr;
    ….
    public: MyFileLogger(const char* aFilename) : theFileStr(aFilename) {}
    ….
    };

    I’m guessing that I should instead write it as:

    class MyFileLogger
    {
    std::ofstream* theFileStr;

    public: MyFileLogger(std::oftream* aFileStr) : theFileStr(aFileStr) {}

    };

    and construct it in the factory method:

    const char* theFilename = “logfile.txt”;

    // might be wise to wrap the ‘std::oftream*’ in a smart-pointer
    // object to manage it’s destruction properly.
    //
    std::ofstream* aFileStr = new std::ofstream(theFilename);

    MyFileLogger theFileLogger(aFileStr);

    I would welcome your advice on whether this is the right approach.

    Cheers,

    Steven

  • misko // Mar 25, 2011 at 8:45 am

    @Steven,

    I am not a C++ expert, but what you are proposing sounds like the right thing to do. I think you are on the right track.

  • David // Apr 1, 2011 at 2:58 am

    Hi Misko, I cannot get my head around clear implementation of lazy loading while adhering to the newable/injectable paradigm. I want my DDD entities to implement transparent lazy loading and to be true newables at the same time. How to achieve this?

    Let’s continue in your example and take the Song as an entity. Song has member properties Song.id (unique song id), Song.loaded (boolean) which indicates if the song has been loaded (e.g. from DB or disk). I also have a field reference to Loader object which does the actual loading (I know this is bad). And finally there are class members for other song properties: song data, author, year, etc.

    Every setter and getter first checks if the object has been loaded and if not it calls this.loader.load(this). This way I have lazy loading transparent to the client and I can use very ‘light’ Song objects in my system, which get loaded only when needed. Until then they are just empty shells with only id set.

    Now I want to factor out the Loader service object (injectable) from the Song (newable) and keep the lazy loading transparent to the client (so that client can call e.g. song.getAuthor() as usual). And I am stuck here. How to invoke the loader service object from getters and setters and not keep a field reference to it in Song? It would be possible to pass the loader to every getter and setter (string Song.getAuthor(Loader loader)) but it seems a lot of hassle and it complicates things for the client.

    Is there a better way? Or am I missing something completely? Thanks for any hint.

    Note 1: Sorry for my terminology and notation. I am a PHP guy and I am just guessing the Java equivalents.

    Note 2: This blog is excellent! Thanks Misko. I wish this kind of knowledge was available also in the PHP world.

  • misko // Apr 1, 2011 at 9:59 am

    @David,

    Your analysis of the problem is very good. What you are asking for is not possible, since new-ables can not have a field reference to an injectable. You have two choices

    1) pass the loading service into each method. (That seems like a bad idea, and puts too much work on the user of your class, and probably breaks Law of Demeter)
    2) change the object from newable to injectable. So the user will say songFactory.create() rather then new Song(). The factory can then set the field.

    There really are no other options, unless you go to the dark side and create a global variable, which is always a bad idea.

  • David // Apr 2, 2011 at 5:21 am

    The Dark Side: I will definitely steer clear of global variables and singletons (and static calls if possible). I’ve learned it the hard way in my tests, that what at first seems like a shortcut is in fact a very painful road to hell.

    The first option (pass the loader into each method) does not appeal to me much for the stated reasons. But I think that the other approach (inject the loader into Song) is flawed as well. Please correct me if I am wrong:

    a) The Song is an Injectable now, but it has state – Seems wrong to me. Is it ok?

    b) The Song does not fit to any DDD category now – The Song class with the Loader field reference is neither a service (it has state and overall it does not look like a service) nor an entity (or is it?). Will this mixed nature of the Song class backfire at some point?

    c) We are breaking our ground rules here – “…Injectable can never ask for a non-Injectable (Newable) in its constructor…”. When the Song was a Newable, we could easily pass primitives or other newables to its constructor to initialize it. But this should be a ‘NO NO’ for Injectables.

    All the three points seem to be intertwined and maybe it’s just three guises of the same issue. Anyway I am baffled. (I hope you say that my conclusions are wrong. Otherwise it would seem that the newable/injectable scheme cannot be fitted to a very simple model?)

  • Bob // Apr 3, 2011 at 4:49 pm

    @David,

    I believe that that injectable option that Misko is describing isn’t to make the Song an injectable but is instead to inject an injectable (in the form of a Song factory) that will then create the Song.

  • misko // Apr 3, 2011 at 9:01 pm

    @David,

    perhaps you could split it into two objects. SongInfo which is newable, and a Song which is also newable but which you get asking the repository

    Song s = songRepository.get(new SongInfo());

    This way you have a clear separation between the object which you can create and which needs to be populated by the system.

  • Lars-Erik Roald // Sep 14, 2011 at 11:28 pm

    Have you considered the factory pattern ?
    And letting having an ISongSetup interface that is only visible to the SongFactory ? The factory itself just resolves directly from the IOC container by calling some Resolve method and then call Setup(arg..) on the Song.

    (eg
    Song : ISongSetup
    ISongSetup : ISong
    ISongFactory)

    If you dont like exposing the IOC Container to the factory, just create your own wrapper around the container.

  • Casey // Mar 28, 2013 at 7:46 am

    Hi,

    I have a long chain like E(D(C(B(A)))))

    E is the interface, A is a provider/factory.

    I need to pass paths and other details to A to trigger the chain resulting in E objects. How do I get a shared_pointer to A from the interface at E?

Leave a Comment