Should I pass an object into a constructor, or instantiate in class?

Question

Consider these two examples:

Passing an object to a constructor

class ExampleA
{
  private $config;
  public function __construct($config)
  {
     $this->config = $config;
  }
}
$config = new Config;
$exampleA = new ExampleA($config);

Instantiating a class

class ExampleB
{
  private $config;
  public function __construct()
  {
     $this->config = new Config;
  }
}
$exampleA = new ExampleA();

Which is the correct way to handle adding an object as a property? When should I use one over the other? Does unit testing affect what I should use?

Answer 1

I think the first one will give you the ability to create a config object elsewhere and pass it to ExampleA. If you need Dependency Injection, this can be a good thing because you can ensure that all intances share the same object.

On the other hand, maybe your ExampleA requires a new and clean config object so there could be cases where the second example is more appropriate, such as cases where each instance could potentially have a different configuration.

Answer 2

Do not forget about testability!

Usually if the behavior of Example class depends on the config you want to be able to test it without saving/modifying the internal state of the class instance (i.e. you would want to have simple tests for happy path and for wrong config without modifying the property/memeber of Example class).

Therefore I would go with first option of ExampleA

Answer 3

If the object has responsibility for managing the lifetime of the dependency, then it's OK to create the object in the constructor (and dispose it in the destructor).*

If the object is not responsible for managing the lifetime of the dependency, it should be passed into the constructor and managed from outside (eg by an IoC container).

In this case I don't think your ClassA should take responsibility for creating $config, unless it is also responsible for disposing it, or if the config is unique for each instance of ClassA.

*To aid in testability, the constructor could take a reference to a factory class/method to construct the dependency in its constructor, increasing cohesion and testability.

//Object which manages the lifetime of its dependency (C#):
public class ClassA : IDisposable
{
    public Config Config { get; private set; }

    public ClassA()
    {
        this.Config = new Config(); // Tightly coupled to Config class...
    }

    public void Dispose()
    {
        this.Config.Dispose();
    }
}

// Object which does not manage its dependency:
public class ClassA
{
    public Config Config { get; set; }

    public ClassA(Config config) // dependency may be injected...
    {
        this.Config = config;
    }
}

// Object which manages its dependency in a testable way:
public class ClassA : IDisposable
{
    public Config Config { get; private set; }

    public ClassA(IConfigFactory configFactory) // dependency may be mocked...
    {
        this.Config = configFactory.BuildConfig();
    }

    public void Dispose()
    {
        this.Config.Dispose();
    }
}

Answer 4

I've had this exact same discussion with our architecture team recently and there are some subtle reasons to do it one way or the other. It mostly comes down to Dependency Injection (as others have noted) and whether or not you truly have control over the creation of the object that you new in the constructor.

In your example, what if your Config class:

a) has non-trivial allocation, such as coming from a pool or factory method.

b) could fail to be allocated. Passing it into the constructor neatly avoids this issue.

c) is actually a subclass of Config.

Passing the object into the constructor gives the most flexibility.

Answer 5

The answer below is wrong, but I'll keep it for others to learn from it (see below)

In ExampleA, you can use the same Config instance across multiple classes. However, if there should be only one Config instance within the entire application, consider applying the Singleton pattern on Config to avoid having multiple instances of Config. And if Config is a Singleton, you could do the following instead:

class ExampleA
{
  private $config;
  public function __construct()
  {
     $this->config = Config->getInstance();
  }
}
$exampleA = new ExampleA();

In ExampleB, on the other hand, you'll always get a separate instance of Config for each instance of ExampleB.

Which version you should apply really depends on how the application will handle instances of Config:

• if each instance of ExampleX should have a separate instance of Config, go with ExampleB;
• if each instance of ExampleX will share one (and only one) instance of Config, use ExampleA with Config Singleton;
• if instances of ExampleX may use different instances of Config, stick with ExampleA.

Why converting Config into a Singleton is wrong:

I must admit that I only learned about the Singleton pattern yesterday (reading the Head First book of design patterns). Naively I went about and applied it for this example, but as many has pointed out, one way are another (some have been more cryptic and only said "You're doing it wrong!"), this is not a good idea. So, to prevent others from making the same mistake that I just did, here follows a summary of why the Singleton pattern can be harmful (based on the comments and what I found out googling it):

1. If ExampleA retrieves its own reference to the Config instance, the classes will be tightly coupled. There will be no way of having an instance of ExampleA to use a different version of Config (say some subclass). This is horrible if you want to test ExampleA using a mock-up instance of Config as there is no way to provide it to ExampleA.

2. The premise of there will be one, and only one, instance of Config maybe holds now, but you cannot always be sure that the same will hold in the future. If at some later point it turns out that multiple instances of Config will be desirable, there is no way of achieving this without rewriting the code.

3. Even though the one-and-only-one instance of Config is maybe true for all eternity, it could happen that you want to be able to use some subclass of Config (while still having only one instance). But, since the code directly gets the instance via getInstance() of Config, which is a static method, there is no way to get the subclass. Again, code must be rewritten.

4. The fact that ExampleA uses Config will be hidden, at least when just viewing the API of ExampleA. This may or may not be a bad thing, but personally I feel that this feels like a disadvantage; for instance, when maintaining, there is no simple way of finding out which classes will be affected by changes to Config without looking into the implementation of every other class.

5. Even if the fact that ExampleA uses a Singleton Config is not a problem in itself, it may still become a problem from a testing point of view. Singleton objects will carry state which will persist until the termination of the application. This can be a problem when running unit tests as you want one test to be isolated from another (i.e. that having executed one test should not affect the outcome of another). To fix this, the Singleton object must be destroyed between each test run (potentially having to restart the whole application), which may be time-consuming (not to mention tedious and annoying).

Having said this, I'm glad that I made this mistake here and not in the implementation of a real application. In fact, I was actually considering rewriting my latest code to use the Singleton pattern for some of the classes. Although I could easily revert the changes (everything is stored in an SVN, of course), I would still have wasted time doing it.

Answer 6

The simplest thing to do is to couple ExampleA to Config. You should do the simplest thing, unless there is a compelling reason to do something more complex.

One reason for decoupling ExampleA and Config would be to improve the testability of ExampleA. Direct coupling will degrade the testability of ExampleA if Config has methods that are slow, complex, or rapidly evolving. For testing, a method is slow if it runs more than a few microseconds. If all the methods of Config are simple and fast, then I would take the simple approach and directly couple ExampleA to Config.

Answer 7

Your first example is an example of the Dependency Injection pattern. A class with an external dependency is handed the dependency by constructor, setter, etc.

This approach results in loosely-coupled code. Most people think that loose coupling is a good thing, because you can easily substitute the config in the cases where one particular instance of an object needs to be configured differently from the others, you can pass in a mock config object for testing, and so on.

The second approach is closer to the GRASP creator pattern. In this case, the object creates its own dependencies. This results in tightly coupled code, this can limit the flexibility of the class and make it more difficult to test. If you need one instance of a class to have a different dependency from the others, your only option is to subclass it.

It can be the appropriate pattern, however, in cases where the lifespan of the depended-on object is dictated by the lifetime of the dependant object, and where the depended-on object is not used anywhere outside of the object that depends on it. Normally I'd advise DI to be the default position, but you don't have to rule out the other approach entirely as long as you're aware of its consequences.

Answer 8

If your class doesn't expose the $config to external classes, then I'd create it inside the constructor. This way, you're keeping your internal state private.

If the $config requires requires its own internal state to be set properly (e.g., needs a database connection or some internal fields initialized) before use, then it makes sense to defer the initialization to some external code (possibly a factory class) and inject it in the constructor. Or, as others have pointed out, if it needs to be shared among other objects.

Answer 9

ExampleA is decoupled from concrete class Config which is good, provided object received if not of type Config but the type of an abstract superclass of Config.

ExampleB is strongly coupled to concrete class Config which is bad.

Instantiating an object creates a strong coupling between classes. It should be done in a Factory class.