2

Problem

I swear, every time I get it pounded into my brain that I should be using virtual calls vs. type-checking (eg:

if (obj is Foo)
   ...
else if (obj is Bar)
   ...

... I come up with another example where I don't know how to implement the former.

I'm implementing a packetized protocol over a serial port. Some pseudo-code will explain this best:

OnDataReceived:
    RcvPacket p = RcvPacket.ReadPacket(comport);   // Call factory method
    if (p is RcvPacketFoo)
        OnFoo();
    if (p is RcvPacketBar)
        OnBar();

OnFoo:
    raise Foo event
OnBar:
    raise Bar event

Basically, ReadPacket is a factory method in the base class that determines the type of packet being received, and passes the buffer to the correct derived-type constructor. After this, I need to raise an event, depending on the type of packet. How can I do this without using the is operator? Is my method sound/sane?


Solution

The Visitor Pattern, of course! Thanks Pablo Romeo.

In this case, I made the controller, that is calling the factory method, is the visitor. My result:

public interface IPacketHandler {
    void Handle(FooPacket p);
    void Handle(BarPacket p);
}

public class Controller : IPacketHandler {
    OnDataReceived() {
        RcvPacket p = RcvPacket.ReadPacket(comport);   // Call factory method
        p.Handle(this);        // *** Virtual Call: The first dispatch ***
    }

    // Note the use of explicit interface implementation here.
    IPacketHandler.Handle(FooPacket p) {
       OnFoo();
    }
    IPacketHandler.Handle(BarPacket p) {
       OnBar();
    }
}

public abstract class RcvPacket {
    public static RcvPacket ReadPacket(...) { ... }   // Factory method
    public void Handle(IPacketHandler handler);
}
public class FooPacket : RcvPacket {
    public override void Handle(IPacketHandler handler) {
       handler.Handle(this);        // *** Overloaded call: The second dispatch ***
    }
}
public class BarPacket : RcvPacket {
    public override void Handle(IPacketHandler handler) {
       handler.Handle(this);        // *** Overloaded call: The second dispatch ***
    }
}

The fun thing here, is that by explicitly implementing the visitor interface, the Handle calls are essentially hidden. From MSDN:

A class that implements an interface can explicitly implement a member of that interface. When a member is explicitly implemented, it cannot be accessed through a class instance, but only through an instance of the interface.

2
  • The class that receives the packet should not be the class that processes it. Once you separate these concerns, it should be obvious how ProcessPacket can be the base class of classes that process packets, instead of RcvPacket. With a virtual OnProcess() method. May 24, 2012 at 23:06
  • @HansPassant I think I follow you... care to provide a small code example? May 24, 2012 at 23:14

2 Answers 2

4

The only way I can think of is to move the implementation of OnFoo and OnBar to the RcvPacketFoo and RcvPacketBar classes.

public class RcvPacket{
     public abstract void On(RcvPacketHandler eh);
}
public class RcvPacketFoo : RcvPacket
{
     public override void On(RcvPacketHandler eh){eh.OnFoo();} //OnFoo implemenation
}

public class RcvPacketBar : RcvPacket
{
     public override void On(RcvPacketHandler eh){eh.OnBar();} //OnBar implemenation
}
//Update following your comment:
public class RcvPacketHandler
{
public void OnFoo(){}
public void OnBar(){}
//....
OnDataReceived:
    RcvPacket p = RcvPacket.ReadPacket(comport);   // Call factory method
    p.On(this);
5
  • If I knew what the implementation of On... was, that may work. Except here, the result of OnFoo() and OnBar() are to raise Foo and Bar events for client code to handle. May 24, 2012 at 23:05
  • Aside from OnFoo and OnBar having to be public, yes. Now I think I'm just getting picky. Also, EventHandler should probably have a different, non-conflicting name. :-P May 24, 2012 at 23:13
  • Unfortunately, I cannot think of a way to preserve the access modifiers. Seems that the different behaviours have to go into the different implementations. I did rename the EventHandler though :). May 24, 2012 at 23:22
  • Thanks for the help. I gave it to Pablo, though - because he specifically points out that this is the visitor pattern / double dispatch. And the use of the interface allows for explicit interface implementation, which hides the On... methods in the handler class. May 24, 2012 at 23:33
  • 1
    Yeah, I think we are both basically saying the same thing using different words. Thanks ;) May 24, 2012 at 23:42
2

Well, considering that you probably don't want much additional logic inside each packet, you can accomplish it through Double Dispatch.

In that case, I'd probably create an interface, such as:

public interface IPacketEvents 
{
    void On(Foo foo);
    void On(Bar bar);
}

I'm assuming you have a base class for all packets. In it I would declare:

public abstract void RaiseUsing(IPacketEvents events);

And each subclass of package would implement the following:

public override void RaiseUsing(IPacketEvents events) 
{
    events.On(this);
}

You could either have a new class implement the IPacketEvents or that same class from where you call the factory could implement it. In that second case, your call would end up as:

OnDataReceived:
    RcvPacket p = RcvPacket.ReadPacket(comport);   // Call factory method
    p.RaiseUsing(this);

Using this type of dispatch, what you get, is that each type, calls the corresponding method, because it "knows" which one to call. It may confuse a bit that I used the same "On" name for all methods, but that's not entirely necessary. They could be OnFoo() and OnBar().

This type of behavior is also used in the Visitor pattern.

2
  • And there it is: My first real use of the Visitor Pattern. I've been wondering for a while where I would use this! The cherry on top is explicitly implementing IPacketEvents (in my class that calls the factory), so that you don't have these strange On(...) public methods hanging out there. May 24, 2012 at 23:28
  • :) Now, one thing to take into account: You don't necessarily need to have them public. You could declare the interface as internal, as well as the RaiseUsing methods. That way, it will only be accessible from your own assembly. May 24, 2012 at 23:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.