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

Sorting Visitors

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

Problem

I started to write a code with top-down tests.
My first version, grow to something like this:

public class Worker
{
  public void Execute(Foo foo)
  {
     //Do X on Foo
     //Do Y on Foo
     //Do Z on Foo

     //Get Bar from Foo

     //Do A on Bar
     //Do B on Bar
     //Do C on Bar
  }
}


then this started to grow more. So, I've to add more actions like X,Y,Z

I refactorized the code in something like:

public interface IFooVisitor
{
  void Visit(Foo foo);
}

public interface IBarVisitor
{
  void Visit(Bar bar);
}

public class Worker
{
  public Worker(
            IEnumerable fooVisitors, 
            IEnumerable barVisitors)
  { ... }
  public void Execute(Foo foo)
  {
    foreach(var fooVisitor in fooVisitors) { fooVisitor.Visit(foo); }
    var bar = getbarfromfoo(foo);
    foreach(var barVisitor in barVisitors) { barVisitor.Visit(bar); }
  }
}


The code is much more cleaner, but the problem is that i need some specific order in the execution of visitors. So, one alternative is to add a property to each visitor like:

public int Priority {get { return 1; } }


The project is opensource and the full code is here:

  • IAbcVisitor is ICloneVisitor



  • IXyzVisitor is IPostWeaveAction

Solution

Have you considered using the Chain of Command instead of the visitor?

public interface IFooChainLink
{
    void Execute(Foo foo);
}

public interface IBarChainLink
{
    void Execute(Bar bar);
}

public class Worker
{
    public Worker(IFooChainLink fooChain, IBarChainLink barChain)
    { ... }

    public void Execute(Foo foo)
    {
        fooChain.Execute(foo);
        var bar = getbarfromfoo(foo);
        barChain.Execute(bar);
    }
}


Your chain links would look like this

public DoXToFoo : IFooChainLink
{
    private static IFooChainLink NextInChain = new DoYToFoo();

    public void Execute(Foo foo)
    {
        // DO STUFF HERE
        NextInChain.Execute(foo);
    }
}


You will also need an EndOfFooChain

public EndOfFooChain : IFooChainLink
{
    public void Execute(Foo foo)
    {
        // DO NOTHING HERE
    }
}


And you might want a start point that makes more sense in your calling code, such as

public FooChainActivator : IFooChainLink
{
    private static IFooChainLink NextInChain = new DoXToFoo();

    public void Execute(Foo foo)
    {
        // DO NOTHING
        NextInChain.Execute(foo);
    }
}


With this, you can have your chain well sequenced and easily insert links into the chain, even the beginning and end without ever changing the calling code.

If you would rather have your chain sequence declared in one place, you can pass NextInChain as a constructor parameter to each layer of the chain, so that your calling code becomes

var worker = new Worker(
                 new DoXToFoo(
                 new DoYToFoo(
                 new DoZToFoo(
                 new EndOfFooChain()))),
                 new DoAToBar(
                 new DoBToBar(
                 new DoCToBar(
                 new EndOfBarChain())))
              );


This has an added advantage that you can send arguments to the links in the chain, but I feel it's a little messier than defining the NextLinkInChain within the links themselves.

I guess that's a matter of personal preference.

Code Snippets

public interface IFooChainLink
{
    void Execute(Foo foo);
}

public interface IBarChainLink
{
    void Execute(Bar bar);
}

public class Worker
{
    public Worker(IFooChainLink fooChain, IBarChainLink barChain)
    { ... }

    public void Execute(Foo foo)
    {
        fooChain.Execute(foo);
        var bar = getbarfromfoo(foo);
        barChain.Execute(bar);
    }
}
public DoXToFoo : IFooChainLink
{
    private static IFooChainLink NextInChain = new DoYToFoo();

    public void Execute(Foo foo)
    {
        // DO STUFF HERE
        NextInChain.Execute(foo);
    }
}
public EndOfFooChain : IFooChainLink
{
    public void Execute(Foo foo)
    {
        // DO NOTHING HERE
    }
}
public FooChainActivator : IFooChainLink
{
    private static IFooChainLink NextInChain = new DoXToFoo();

    public void Execute(Foo foo)
    {
        // DO NOTHING
        NextInChain.Execute(foo);
    }
}
var worker = new Worker(
                 new DoXToFoo(
                 new DoYToFoo(
                 new DoZToFoo(
                 new EndOfFooChain()))),
                 new DoAToBar(
                 new DoBToBar(
                 new DoCToBar(
                 new EndOfBarChain())))
              );

Context

StackExchange Code Review Q#677, answer score: 4

Revisions (0)

No revisions yet.