How can I avoid violation of Open Closed with method parameters?

Question

In the code below the MessageProcessor class is violating Open Closed principle - every new IMessage implementation requires a change to this class. Is there a nice clean pattern for this kind of scenario that doesn't violate O/C?

public interface IMessage
{
}

public class BlahMessage : IMessage
{
}

public class MoohMessage : IMessage
{
}

public class MessageStream
{
    public void Dispatch(IMessage message)
    {
        var messageProcessor = new MessageProcessor();
        messageProcessor.Handle(message);
    }
}

public class MessageProcessor
{
    public void Handle(IMessage message)
    {
        if (message is MoohMessage)
            Handle((MoohMessage)message);

        if (message is BlahMessage)
            Handle((BlahMessage)message);
    }

    private void Handle(MoohMessage moo)
    {
    }

    private void Handle(BlahMessage blah)
    {
    }
}

Other than the signature, how are `Handle(MoohMessage moo)` and `Handle(BlahMessage blah)` different? Should those be in separate classes (possibly generic)? — D Stanley, May 22 '14 at 13:03
Why do the `Handle` methods need to be different? At first glance it looks like that logic should be on the messages themselves and `MessageProcessor` should just call a generic method on `IMessage` to invoke it. — David, May 22 '14 at 13:04
Why not add Process() method to IMessage and call *message.Process()* instead of *messageProcessor.Handle(message)*? — MVarman, May 22 '14 at 13:08
Messages are just dumb DTOs, they are messages after all, they aren't going to have a Process() method on themselves. — glasswall, May 22 '14 at 14:21

score 2 · Answer 1 · answered May 22 '14 at 13:18

2

The problem here is, MessageProcessor class trying to implement the behaviour of different Message classes.

Instead, you could add Process() method to IMessage and implement it in each Message classes.

So the interface looks like,

public interface IMessage
{
   Process();
}

and, Dispatch method can call this method directly

public void Dispatch(IMessage message)
{
  message.Process();
}

answered May 22 '14 at 13:18

MVarman

549
2
9

But that violates SRP, which is arguably worse. Messages are just that - messages. They don't contain the logic required to process themselves, that doesn't even make sense. – glasswall May 22 '14 at 14:18
I didn't realize Message objects were plain DTOs. If that is the case, the other option is, Create IMessageProcessor and implement IBlahMessageProcessor and IMoohMessageProcessor. You can then either inject IMessageProcessor object to Dispatch() method or use Factory pattern to create the Processor object (if you dont mind [factory class violating OCP](http://java.globinch.com/patterns/design-patterns/factory-design-patterns-and-open-closed-principle-ocp-in-solid/)). – MVarman May 22 '14 at 14:46
Sorry, I should have been clearer.. Yeah I guess at some point, unless you use a DI container that autowires the handlers you're going to violate SRP... thanks :) – glasswall May 22 '14 at 14:57

score 0 · Answer 2 · answered May 22 '14 at 19:07

Isn't your current MessageProcessor violating SRP because it processes multiple kinds of messages?

Here's an option to consider:

Change the MessageStream to send messages to a Dispatcher.
Create distinct message processing classes.
Register each message processor with the Dispatcher at runtime.
The Dispatcher will route messages to the correct Processor.

Of course this approach may just shift the OCP problem to another place where MessageProcessor instances are created and regitered with the Dispatcher. But at least it would clean up the many if (message is XXX) calls.

How can I avoid violation of Open Closed with method parameters?

2 Answers2