1

I'm trying to exercise the principles of Dependency Injection, and I'm having some difficulty doing so.

I have a function that periodically goes off to my database, retrieves a list of products, and then runs a battery of tests against those products in order to determine their safety.

If one or more of the products is found to be unsafe, I need to issue a recall by instantiating and dispatching a ProductRecall object.

The function looks something like this: (pseudo-code)

void SafteyInspector::CheckProductSaftey()
{
  database.getProducts( list_of_products )

  foreach( list_of_products as p )
  {
    if ( !runBatteryOfTests( p ) )
      unsafe_products.insert( p );
  }

  if ( !unsafe_products.empty() )
  {
    ProductRecall * recall = 
          new ProductRecall( unsafe_products );  // Oh no!
    recall->dispatch();
  }
}

The problem is that I'm "new"-ing a ProductRecall object right in the middle of my call-graph. This violates the principles of Dependency Injection. As written, I can't test CheckProductSaftey() without also creating a ProductRecall object.

However, I can't pass the ProductRecall object into my SafetyInspector object, because the SafetyInspector is the one who determines the list of unsafe products.

I'm using constructor-injection for everything, and I'd like to continue doing so. Note also that I may issue multiple ProductRecalls at any time, so I can't necessarily just pass a single ProductRecall object into the SafetyInspector at construction.

Any suggestions? Thanks!

ire_and_curses
  • 68,372
  • 23
  • 116
  • 141
Runcible
  • 7,006
  • 12
  • 42
  • 62
  • You had your code embedded in 
     tags as opposed to having it code formatted (four spaces before each line). I switched it for you so we can have the benefit of SO's pretty code coloring, but if your choice was intentional, please feel free to change it back.
     – e.James Apr 27 '09 at 21:39
  • Ah, ha! I've been wondering how that works. Thanks for the tip! – Runcible Apr 27 '09 at 21:58
  • No problem. When I first started here, I manually embedded everything in tags. Live & learn! :) – e.James Apr 27 '09 at 22:41

2 Answers2

1

I think you actually need to conjure up some sort of ProductRecallFactory to pass in instead. It's fairly common for injection containers to support some sort of factory-style interface, or you may just wish to make your SafetyInspector container-aware.

EDIT: by "container-aware" I'm referring to implementing some interface along the lines of COM's IObjectWithSite, so that your object can call back to its parent. It's a weaker form of dependency injection which partially undoes the inversion of control. If you're doing the dependency injection manually, by all means inject a factory object.

Jeffrey Hantin
  • 35,734
  • 7
  • 75
  • 94
1

I think the problem may be with your implementation of ProductRecall. Specifically, if you can call dispatch() on a newly created object, it implies that a lot of actual functionality is either hidden inside the ProductRecall class, or that the ProductRecall class has static members and/or singletons to give it everything else it needs.

I would recommend creating a class called ProductRecallDispatcher which handles the intricacies of an actual dispatch. You would then create one of these objects, and pass it to the constructor for SafteyInspector. This would make your CheckProductSafety function look as follows:

void SafteyInspector::CheckProductSaftey()
{
  database.getProducts( list_of_products )

  foreach( list_of_products as p )
  {
    if ( !runBatteryOfTests( p ) )
      unsafe_products.insert( p );
  }

  if ( !unsafe_products.empty() )
  {
    recallDispatcher.recall( unsafe_products );
  }
}
e.James
  • 116,942
  • 41
  • 177
  • 214
  • Hmm. You're definitely correct that there is quite a bit of complexity inside the ProductRecall object (and as you guessed, it does rely on a couple of singleton objects). That being said -- if I create a ProductRecallDispatcher class it seems like I'm going to have the same problem inside this new object. Specifically, I'll have to "new" a ProductRecall object from inside ProductRecallDispatcher::recall(). – Runcible Apr 27 '09 at 22:07
  • It will depend on what the .recall() function actually does. I would suggest that most of that functionality should be moved into the dispatcher, and that a ProductRecall holds only the data associated with the actual recall, (and functions to aid in its creation). The "new"-ing would be less of a problem, since the RecallDispatcher would still own the recall object (perhaps by keeping an STL vector of pointers), and could safely dispose of it at destruction. – e.James Apr 27 '09 at 22:37
  • Incidentally, those singletons can be a problem, since using them prevents you from doing proper dependency injection. http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons/138012#138012 – e.James Apr 27 '09 at 22:39
  • Refactoring to remove singletons and switch to dependency injection is a fun job. Good luck! :) – e.James Apr 27 '09 at 22:40