0

Possible Duplicate:
Dependency Hell — how does one pass dependencies to deeply nested objects?

In a system built around strong dependency injection, I'm wondering how to deal with a contrived situation like this:

<?php
class LogWriter
{
    public function write(Log $log)
    {
        echo $log->getMessage();
    }
}

class Log
{
    private $message;
    public function setMessage($message)
    {
        $this->message = $message;
    }
    public function getMessage()
    {
        return $this->message;
    }
}

class Logger
{
    private $writer;
    public function __construct(LogWriter $writer)
    {
        $this->writer = $writer;
    }
    public function write($message)
    {
        // Here is the dependency
        $log = new Log();
        $log->setMessage($message);
        $this->writer->write($log);
    }
}

The Logger::write() method creates an instance of Log, and passes it to the log writer. My gut tells me that's a bad approach, and a month from now I'm going to be tracking down a bug related to it, and I might want to switch the Log class for something else during testing.

But how to avoid it? The only thing that comes to mind is passing a Log type to the Logger constructor, and changing my Logger class to this:

class Logger
{
    private $writer;
    private $log_type;
    public function __construct(LogWriter $writer, $log_type)
    {
        $this->writer = $writer;
        $this->log_type = $log_type;
    }
    public function write($message)
    {
        $log = new $this->log_type();
        $log->setMessage($message);
        $this->writer->write($log);
    }
}

And then creating a new Logger instance like this:

$log_writer = new LogWriter();
$logger = new Logger($log_writer, "Log");

But that feels a bit hackish. So how do you deal with micro-dependencies like this?

Note: I'm using the logging classes as an example, and I'm not looking for a solution to this exact problem. I would probably just use an array instead of the Log class.

Edit: In a more complex situation, I might pass a dependency injection container to the Logger class, and use that to create an instance of Log, but that seems overly complicated for a simple logger class.

Community
  • 1
  • 1
mellowsoon
  • 22,273
  • 19
  • 57
  • 75

1 Answers1

3

Since your Log object is really just a Data Transfer Object or Value Object, you can create it inside the Logger class. It's okay to do so in this case. You dont need to pass anything to the Logger. But you are right in that you wont be able to mock/stub this easily then.

As an alternative, you could also inject a Factory if you want to decouple the Log class from the Logger:

$logger = new Logger($logWriter, new LogFactory);

and then create the Log Type from there:

public function write($message)
{
    $log = $this->logFactory->createNew();
    …

This capsules the creation logic inside the Factory class. The Factory will still have the Log type hardcoded inside, but it's okay for Factories to have that. You then just test that it returns the right type when you call createNew. And in your consumers, you can stub that call.

If you dont feel like creating a full-blown Factory class for this, you can also use a Lambda instead of a Factory. Since it captures the essential creation logic, it's effectively the same as a Factory, just without a class:

$logger = new Logger($logWriter, function() { return new Log; });

And then

public function write($message)
{
    $log = call_user_func($this->createLogCallback);
    …

Both, the Factoy and the Lambda approach allow for substituting the Log type in your Unit-Test. Then again, substituting the Log type doesn't seem that necessary in your scenario. The Log type doesn't have any dependencies of it's own, so you can pretty much use the real deal here. You can easily verify your write method by simply looking at what gets written by the LogWriter. You won't have an explicit assertion on a Log Mock, but if the writer produces the expected output for the given input to write, you can safely assume that the Log type collaborates as expected.

Also see http://misko.hevery.com/2008/09/30/to-new-or-not-to-new for more details.

Gordon
  • 312,688
  • 75
  • 539
  • 559
  • Question: What would you do? Pass a factory to the constructor, or just say, "Eh, it's only a value object. I can use 'new' here."? – mellowsoon Sep 09 '12 at 21:44
  • 1
    @mellowsoon: Do whatever works best now -- in your case, it probably means sticking with `new` for the time being. Don't think about what you might need in the future, [you ain't gonna need it](http://en.wikipedia.org/wiki/You_ain%27t_gonna_need_it). – casablanca Sep 09 '12 at 23:37
  • @mellowsoon Personally, I like the Lambda approach because of it's simplicity. Then again, I dont see much value in being able to replace the Log type, so I'd likely just `new` the `Log`. See the added section at the end for more details. – Gordon Sep 10 '12 at 07:25