Using marker classes to control logic flow

Question

I have been going through some code seen that a colleague of mine is using 'marker classes' to control program logic (see contrived example below). It seems to work well, and the code reads really nicely, but there is just something about it that smells...

namespace ConsoleApplication4983
{
    public class MyClass
    {
        static void Main()
        {
            var c = new MyClass();
            c.DoSomething(new Sequential());
            c.DoSomething(new Random());
        }

        public void DoSomething(ProcessingMethod method)
        {
            if (method is Sequential)
            {
                // do something sequential
            }
            else if (method is Random)
            {
                // do something random
            }
        }
    }

    public class ProcessingMethod {}
    public class Sequential : ProcessingMethod {}
    public class Random : ProcessingMethod {}
}

What would be a better way of achieving the same effect? Enums? Attributes?

Answer 1

Marker interfaces are a better practice as they offer much more flexibility.

However in this specific case I think that virtual dispatch is a better solution.

using System;

namespace ConsoleApplication4983
{
    public class MyClass
    {
     static void Main()
     {
      var c = new MyClass();
      c.DoSomething(new Sequential());
      c.DoSomething(new Random());
     }

     public void DoSomething(ProcessingMethod method)
     {
      method.Foo();
     }
    }

    public class ProcessingMethod
    {
     public virtual void Foo() { }
    }
    public class Sequential : ProcessingMethod
    {
     public override void Foo() { }
    }
    public class Random : ProcessingMethod
    {
     public override void Foo() { }
    }
}

Answer 2

He was almost there, but not quite, and that's probably what you're seeing. The if statement on the type is the bad smell. The do something should have been on the ProcessingMethod base class and each type that extended it should have their own version.

public void DoSomething(ProcessingMethod method)    {
   method.DoSomething();
}

Answer 3

How about delegating the processing logic to the specific subclass? ProcessingMethod would have some abstract method that is implemented by each subclass.

public void DoSomething(ProcessingMethod method)
{
  method.Process();
}

public abstract class ProcessingMethod
{
  public abstract void Process();
}

public class Sequental : ProcessingMethod
{
  public override void Process()
  {
    // do something sequential
  }
}

public class Random : ProcessingMethod
{
  public override void Process()
  {
    // do something random
  }
}

Answer 4

Yeah, this smells bad. If you want to do something parallel:

public class Parallel : ProcessingMethod{}

then you're going to have to change a lot of code.

Answer 5

What you'd like to do is replace this with a strategy pattern. A strategy defines how something is done -- i.e., an algorithm.

public interface IProcessingMethod
{
    void Process();
}

public class SequentialProcess : IProcessingMethod
{
    public void Process( IProcessable obj )
    {
         do something sequentially with the obj
    }
}

public class ParallelProcess : IProcessingMethod
{
    public void Process( IProcessable obj )
    {
        do something in parallel with the obj
    }
}

public interface IProcessable
{
    void Process( IProcessingMethod method );
}

public class MyClass : IProcessable
{
     public void Process( IProcessingMethod method )
     {
         method.Process( this );
     }
}

...

var obj = new MyClass();
obj.Process( new SequentialProcess() );

Now if I have a new type of ProcessingMethod, I simply need to create the class for that method and change the code that determines what processing method is injected to the Process method of my IProcessable object.

Answer 6

The Framework Design Guidelines book recommends against using marker interfaces (and presumably marker classes), preferring attributes intead. Having said that, the book does go on to say that using is (as you've done) is much quicker than using reflection to check for an attribute.