How can a switch statement be avoided if the return type is unknown by the calling method?

Question

I have been reading a book called Clean Code A Handbook of Agile Software Craftsmanship. The author in the book motivates that a switch statement should be avoided and if it cannot be avoided it should be relegated to factory methods. I have a connection object which is receiving various PDUs (protocol data units). The PDUs vary and they can be received in any order. So if I have a method for example:

public BasePdu ReceiveNext();

because I cannot tell what the packet type is until it has been fully received. In the header of the PDU is an identifier as to which type it should be. This means that the calling method is going to have figure out what the type of the PDU is and based on that call the relevant method to handle it. This sounds like a perfect example for a switch statement. The object that contains the connection I would ideally like to have two threads. One for receiving PDUs and another for servicing a queue of PDUs to be sent.

Now I know that you cannot follow every bit of good advice and that there are just some circumstances which are the exception to the rule. Is this one of them? Or is there a way around this that I just have not yet thought of.

UPDATE:

I hear what a lot of people are saying by creating subclasses of response handlers. The issue is that the containing object has a lot of context and additional information that the handlers would need for example lookups and throttling etc etc. To inject all of this information into subclasses of handlers would be quite a chore to maintain and would also split a lot of logic up when it feels better to be encapsulated in the object that it is in now.

Answer 1

Simply create a PDUParserFactory which create the parser based on a PDU type using switch statements on the PDU type identifier. This is the case where the book says it's ok :)

Update: One possible approach

 class BasePDU
 {
     string Name { get; set; }
     ...
 }

 class PDUType1 : BasePDU
 {
     ...
 }
 ...

 class PDUReceiver
 {
     public event EventHandler<PDUReceivedEventArgs> PDUReceived;

     private void ParsePDU(byte[] data)
     {
          BasePDU pdu;
          switch (byte[0]) // PDU type selector
          {
              ....    parse PDU based on type
          }

          OnPDUReceived(pdu);
     }

      private void OnPDUReceived(BasePDU pdu)
      {
           var handler = PDUReceived;
           if (handler != null)
           {
                handler(this, new PDUReceivedEventArgs(pdu));
           }
      }
 }

Then you can attach listeners to the event:

 pduReceiver.PDUReceived += BaseHandler;
 pduReceiver.PDUReceived += PDUType1Handler;
 ...

 void PDUType1Handler(object sender, PDUReceivedEventArgs e)
 {
      // only care about PDUType1
      if (e.PDU.GetType() != typeof(PDUType1))
            return;
      ....
 }

Alternatively you could also create a event handler dictionary in the receiver, mapping a pdu type to event handlers and then let handlers register for specific types only. This way not all handlers would be called for each received PDU.

Instead of heaving a PDU type hierarchy you could also just have a:

 class PDU
 {
      public PDUType PDUType { get; }
      public byte[] PDUData { get }
 }

then register handlers in the receiver for each PDUType and let the handler do whatever it wants with the data.

It's hard to give more concrete advice without knowing what exactly you want to do with your received packets.

Answer 2

Personally I wouldn't worry about it too much; if it looks like a good place for a switch statement use one. On the other hand this also looks like a situation where you could use a factory method if each PDU type is handled by a class rather than a method. And, accoding to your book, you're allowed you to use switch statements then

Answer 3

The reason to avoid swith statements are not because if structures are any better (when a switch is used, a bunch of if's will make it worse, not better) it mainly because the problem is not solved in an OO way.

From an OO point of view it is almost always better to use polymorphism then a switch statement.

In your example it's probably better to use a factorymethod to provide the appropriate handler for your type of package.

Answer 4

If I understand your question correctly you have really two questions:

---

How to create the correct PDU's when you've received the name without using switch.

Create a simple factory by using a dictionary Dictionary<string, Func<PduBase>>

---

How the method calling public BasePdu ReceiveNext(); can handle it properly without using switch

Do not use a RecieveNext method. Create a AddPduHandler<T>(IPduHandler<T> handler) where T : PduBase method to the class receiving all PDU's. Store all handlers in a dictionary with the type as key: Dictionary<Type, Delegate>

Storing a delegate is kind of a trick since you can not work with the typed interface in the receiving class.

Update

This solution do not break Liskovs Substitution Principle which all implementations using switch do. This means that this class will work no matter how many different types of PDU's that you have.

It's also easier to test your application since each handler is isolated from everything else.

The bonus side is that everything is Typed (except the reader class) which will make it easier of find errors instead of working with casting magic or such.

    public class Receiver
    {
        Dictionary<Type, MethodMapping> _handlers = new Dictionary<Type, MethodMapping>();
        Dictionary<string, Func<PduBase>> _factories = new Dictionary<string, Func<PduBase>>();

        // Small container making it easier to invoke each handler
        // also needed since different generic types cannot be stored in the same 
        // dictionary
        private class MethodMapping
        {
            public object Instance { get; set; }
            public MethodInfo Method { get; set; }
            public void Invoke(PduBase pdu)
            {
                Method.Invoke(Instance, new[] {pdu});
            }
        }

        // add a method used to create a certain PDU type
        public void AddFactory(string name, Func<PduBase> factoryMethod)
        {
            _factories.Add(name, factoryMethod);
        }

        // register a class that handles a specific PDU type
        // we need to juggle a bit with reflection to be able to invoke it
        // hence everything is type safe outside this class, but not internally.
        // but that should be a sacrifice we can live with.
        public void Register<T>(IPduHandler<T> handler) where T : PduBase
        {
            var method = handler.GetType().GetMethod("Handle", new Type[] { typeof(T) });
            _handlers.Add(typeof(T), new MethodMapping{Instance = handler, Method = method});
        }

        // fake that we've received a new PDU
        public void FakeReceive(string pduName)
        {
           // create the PDU using the factory method
            var pdu = _factories[pduName]();

            // and invoke the handler.
            _handlers[pdu.GetType()].Invoke(pdu);
        }
    }

    public interface IPduHandler<in T> where T: PduBase
    {
        void Handle(T pdu);
    }

    public class TempPdu : PduBase
    {}

    public class TempPduHandler : IPduHandler<TempPdu>
    {
        public void Handle(TempPdu pdu)
        {
            Console.WriteLine("Handling pdu");
        }
    }

    public class PduBase
    { }



    private static void Main(string[] args)
    {
        Receiver r = new Receiver();
        r.AddFactory("temp", () => new TempPdu());
        r.Register(new TempPduHandler());

        // we've recieved a PDU called "temp". 
        r.FakeReceive("temp");
    }

Answer 5

not sure if that's exactly the point, but having different instances whose treatment differs according to an ID is in fact a case for creating subclasses (the choice of subclass representing the information that was previously stored in the ID) of eg BasePdu and have the compiler figure out which method to use. if you're doing that by switching, it means you're not fully taking advantage of structuring your code by subclassing.