Question

I've been trying to follow the principles of Dependency Injection, but after reading this article, I know I'm doing something wrong.

Here's my situation: My application receives different types of physical mail. All the incoming mail passes through my MailFunnel object.

While it's running, MailFunnel receives different types of messages from the outside: Box, Postcard and Magazine.

Each mail type needs to be handled differently. For example, if a Box comes in, I may need to record the weight before delivering it. Consequently, I have BoxHandler, PostcardHandler and MagazineHandler objects.

Each time a new message comes into my MailFunnel, I instantiate a new corresponding MailHandler object.

For example:

class MailFunnel
{
  void NewMailArrived( Mail mail )
  {
    switch (mail.type)
    {
      case BOX:
        BoxHandler * bob = new BoxHandler(shreddingPolicy, maxWeightPolicy);
        bob->get_to_work();
        break;

      case POSTCARD:
        PostcardHandler * frank = new PostcardHandler(coolPicturePolicy);
        frank->get_to_work();
        break;

      case MAGAZINE:
        MagazineHandler * nancy = new MagazineHandler(censorPolicy);
        nancy->get_to_work();
        break;
    }
  }

  private:
    MaxWeightPolcy & maxWeightPolicy;
    ShreddingPolicy & shreddingPolicy;
    CoolPicturePolicy & coolPicturePolicy;
    CensorPolicy & censorPolicy;
}

On one hand, this is great because it means that if I get five different pieces of mail in, I immediately have five different MailHandlers working concurrently to take care of business. However, this also means that I'm mixing object creation with application logic - a big no-no when it comes to Dependency Injection.

Also, I have all these policy references hanging around in my MailFunnel object that MailFunnel really doesn't need. The only reason that MailFunnel has these objects is to pass them to the MailHandler constructors. Again, this is another thing I want to avoid.

All recommendations welcome. Thanks!

Was it helpful?

Solution

This looks more like a factory to me. Move the invocation of the get_to_work() method out of the invocation and return the handler. The pattern works pretty well for a factory.

class MailHandlerFactory
{
  IMailHandler*  GetHandler( Mail mail )
  {
    switch (mail.type)
    {
      case BOX:
        return new BoxHandler(shreddingPolicy, maxWeightPolicy);
        break;

      case POSTCARD:
        return new PostcardHandler(coolPicturePolicy);
        break;

      case MAGAZINE:
        return new MagazineHandler(censorPolicy);
        break;
    }
  }

  private:
    MaxWeightPolcy & maxWeightPolicy;
    ShreddingPolicy & shreddingPolicy;
    CoolPicturePolicy & coolPicturePolicy;
    CensorPolicy & censorPolicy;
}

class MailFunnel
{
    MailHandlerFactory* handlerFactory;

    MailFunnel( MailHandlerFactory* factory ) {
        handlerFactory = factory;
    }

    void NewMailArrived( Mail mail ) {
        IMailHandler handler = handlerFactory.GetHandler(mail);
        handler.get_to_work();
    }
}

OTHER TIPS

When you see that switch statement, think polymorphism. This design can only be extended by modification. I'd redo it in such a way that I could add new behavior by adding classes. It's what the open/closed principle is all about.

Why can't you just have three methods that are overloaded which take the different types of mail, and then do the appropriate thing? Or have each type handle itself.

In fact, if you have something like type, chances are you should in fact have different types.

Basically do the following:

1) Make the Mail class abstract.

2) Create a three sub classes of mail, Box, PostCard, and Magazine

3) Give each subclass a method to handle mail, or centralize it in a separate HandlerFactory

4) When passed to the mail funnel, simply have it call the handle mail method, or have the HandlerFactory pass it the mail, and get the appropriate handler back. Again, rather than having awkward switch statements everywhere, use the language, this is what types and method overloading is for.

If your mail handling becomes complex and you want to take it out, you can eventually make a mail handler class and extract those policies into that.

You can also consider using a template method, because the only real difference between each of those seems to be the handler you instance, maybe you could simplify it, such that the mail type determines the handler, the rest of the code is basically the same.

Interesting that you're applying dependency injection to a C++ project; it has been done elsewhere, a quick Google search finds a Google code project Autumn Framework.

But the answer by tvanfosson is what I would suggest trying first, before adopting a new framework.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top