Reputation: 7372
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!
Upvotes: 4
Views: 1679
Reputation: 3413
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.
Upvotes: 2
Reputation: 14658
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.
Upvotes: 1
Reputation: 532595
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();
}
}
Upvotes: 8
Reputation: 308938
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.
Upvotes: 2