4
votes
\$\begingroup\$

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<IFooVisitor> fooVisitors, 
            IEnumerable<IBarVisitor> 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
\$\endgroup\$

locked by Jamal Aug 9 '14 at 23:44

This question exists because it has historical significance, but it is not considered a good, on-topic question for this site so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. See the help center for guidance on writing a good question.

Read more about locked posts here.

5
votes
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ I like the second pattern better. The first pattern is more fragile (imagine inserting a new link badly, and the chain missing out important steps) and can't be manipulated at run time. \$\endgroup\$ – tenpn Feb 8 '11 at 9:29
  • \$\begingroup\$ Maybe a third option would be a factory that produced the chain for you to insert into the worker. That way you can still hard-code stuff but can also chop and change at run time. \$\endgroup\$ – tenpn Feb 8 '11 at 9:29
  • \$\begingroup\$ Thank you for your answer, still not convinced: 1-I don't like the fist aproach because it requires to hard code the next visitor inside the class and the instantiaton. 2-the second aproach seems better but i feel it is not dependency injection friendly, every visitor has his own set of dependencies injected by an DI container (well.... it is MEF). \$\endgroup\$ – José F. Romaniello Feb 8 '11 at 12:43
  • \$\begingroup\$ @Jose - It doesn't really need to be DI container friendly. You can test your Worker class by passing it a mock IFooChainLink and IBarChainLink and each of your chain links independantly using a similar approach (with just one mock). That's the beauty of it. \$\endgroup\$ – pdr Feb 8 '11 at 17:23

Not the answer you're looking for? Browse other questions tagged or ask your own question.