7

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?

7 Answers 7

8

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() { }
    }
}
Sign up to request clarification or add additional context in comments.

Comments

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.

Comments

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();
}

Comments

2

I see that this question is old, but I feel that all the answers missed the point.

If the example fully illustrates the extent of the required functionality, then the appropriate construct to use here would be an Enum type. Enum types are value types; they function essentially like named numerical constants, with great IDE autocomplete support. Here is the example modified to use an Enum type:

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

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

    public enum ProcessingMethod
    {
        Sequential,
        Random
    }
}

The other answers are making reference to more elaborate patterns. I think they read too much into the term "marker class". Sometimes strategy pattern, virtual dispatch etc. are a good way to go, but in this case I think an Enum is the simplest improvement to be made to this code.

Comments

0

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
  }
}

Comments

0

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.

Comments

0

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.

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.