HiveBrain v1.2.0
Get Started
← Back to all entries
principlecsharpMinor

Type checking to perform a variation of the strategy pattern

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thestrategycheckingtypeperformvariationpattern

Problem

So, consider the following code:

public class OneConcreteTypeOfOutputFormat 
{
    private Dictionary writers;
    public Dispatcher()
    {
        this.writers = new Dictionary writers)
    {
        string nspace = typeof(OneConcreteWriter).Namespace;
        Assembly assembly = Assembly.GetExecutingAssembly();
        foreach (var writer in assembly.GetAllFromNamespace(nspace))
            writers.Add(writer.EntityType.FullName, writer);
    }

    private bool HasWriterForAllPropTypes(IEnumerable data)
    {
        return data.All(ent => writers.ContainsKey(ent.GetType().FullName));
    }

    public void WriteEntities(IEnumerable entities)
    {
         Require.That(() => HasWriterForAllPropTypes(outputData));
         foreach(Entity entity in entities)
             writers[entity.GetType().FullName].Write(entity);
    }
}


Here, I am using the actual type of the Entity (which is a baseclass) to write it. I've heard that it's considered inelegant to use type-checking to perform operations, but I really don't want each Entity to write itself. This is a very easy and comfortable way to match something like this up. Do you find this clashing with the tenets of object-oriented programming? Sadly, each writer need to cast to it's specific type in the Write method.

UPDATE

Note that the code above is completely updated - changes include:

-
Added writer initialization code. It is delegated to a unit-tested extension method.

-
Changed the name of the class to reflect that it is only one of different types of output formats - so it would require more than just creating one writer for each Entity.

-
Added a little bit of the validation code.

Solution

I don't know if it will be useful for you, but here is one variant:

Add to your Entity base class a method called GetWriter. This method should return IEntityWriter.
This IEntityWriter should have method Write() without parameters.

Then an implementation of an Entity would be something like:

class XXXEntity: Entity
{
   public override IEntityWriter GetWriter()
   {
      return new XXXEntityWriter(this);
   }
}


And then the method WriteEntities in your example would be

public void WriteEntities(IEnumerable entities)
{
   foreach(Entity entity in entities)
      entity.GetWriter().Write();
}


The 'mappings' approach (no matter if it is in the code or in the xml file) have a couple of problems:

  1. It's easy to do wrong configuration of mappings. You can map a XXXEntity to YYYEntityWriter.



  1. It's easy to forget to add an EntityWriter when you add new Entity or you can forget to add the mapping. And this is a real problem.



By the way. Judging from the description of the task, a concrete EntityWriter have to know about a concrete Entity.
So mapping is not as flexible as it seems, because of this explicit connection between EntityWriter and Entity.
It's not a problem to use GetType(). The problem here is to separate (hide) connections between classes that should work together.

Code Snippets

class XXXEntity: Entity
{
   public override IEntityWriter GetWriter()
   {
      return new XXXEntityWriter(this);
   }
}
public void WriteEntities(IEnumerable<Entity> entities)
{
   foreach(Entity entity in entities)
      entity.GetWriter().Write();
}

Context

StackExchange Code Review Q#3689, answer score: 6

Revisions (0)

No revisions yet.