Wednesday, August 16, 2006

Why visitor pattern is worth the overhead

I've observed that many developers tend to shy away from the visitor pattern. I suspect it's because even though they understand the pattern, the double-dispatch and the seemingly out-of-place accept methods seem like too much overhead. So, instead of using visitor, we end up with logic that switches on a simple enumeration or the type of an object. Compared to visitor, this kind of code is arguably simpler to understand.

I think that visitors are worth the overhead. What we gain is the ability to easily identify the effects of type changes.

Let's use this example: One part of our application captures user feedback. They have two options:
  • Choose a feedback from a standard list (eg. "Good" or "Bad")
  • Submit a short sentence describing their usage experience.


// C# pseudocode...

abstract class Feedback
{
    void Accept(IFeedbackVisitor visitor);
}

class StandardChoiceFeedback : Feedback
{
    int Code;
    string Description;
    void Accept(IFeedbackVisitor visitor) { visitor.Visit(this); }
}

class FreeFormFeedback : Feedback
{
    string Text;
    void Accept(IFeedbackVisitor visitor) { visitor.Visit(this); }
}

interface IFeedbackVisitor
{
    void Visit(StandardChoiceFeedback feedback);
    void Visit(FreeFormFeedback feedback);
}


Somewhere in the system, we display a grid of data, where each row corresponds to a user and one of the columns shows their feedback:

class FeedbackColumnRenderer : IFeedbackVisitor
{
    string RenderedText;

    void Visit(StandardChoiceFeedback feedback)
    {
        RenderedText = feedback.Description;
    }

    void Visit(FreeFormFeedback feedback)
    {
        RenderedText = feedback.Text;
    }
}

string RenderFeedbackColumn(Feedback feedback)
{
    FeedbackColumnRenderer renderer = new FeedbackColumnRenderer();
    feedback.Accept(renderer);
    return renderer.RenderedText;
}


So far, all we've shown is the overhead incurred by using visitor.

However, what happens when the requirements change? For example, our business analyst comes back in 3 weeks to inform us that on top of the two feedback options, there needs to be another option — the user can choose to leave their phone number so that a customer representative can give them a follow-up call to chat about their experience. We are asked to implement this change in the UI and the persistence layer.

Our core types change like this:

class FollowUpCallFeedback : Feedback
{
    PhoneNumber phoneNumber;
    void Accept(IFeedbackVisitor visitor) { visitor.Visit(this); }
}

interface IFeedbackVisitor
{
    void Visit(StandardChoiceFeedback feedback);
    void Visit(FreeFormFeedback feedback);
    void Visit(FollowUpCallFeedback feedback);
}


With just these changes, let's re-compile. What we find is that compilation isn't successful because all implementors of IFeedbackVisitor now have to implement the new Visit(FollowUpCallFeedback) method. On further examination of the compiler errors, we discover that not only are there feedback visitors in the UI and persistence layers, but there is also one used for an obscure report. We talk to our business analyst again, and find out that this report has been missed in talking about the change. Great! We then talk about what the report should look like when a phone number is involved.

I think this illustrates the power of the pattern. When something changes in our type hierarchy, we are actually forced to address all the places which will be affected up-front. If we had chosen to use a simple enumeration or type switching scheme instead and we are not extremely careful, we might not discover all the places that are affected by the requirements change until much later (for instance, when the testers [or worse, the users] visually inspect that particular report).

4 comments:

Anonymous said...

I think this is a great way to show how decoupling in general is a powerful tool in software design. My problem with Visitor is that there certainly IS complexity involved in using it, and I feel that it is sometimes overused. My opinion is that it should be used in algorithms which form the "backbone" of the design because that's where the decoupling will save you time later. It should NOT be used every time you have to iterate through a list. Not that you're saying that or anything.

Anonymous said...

however the visitor pattern remain coupled in one direction(visitor needs to know visitable concrete objects). A way to reduce it is to use reflection like here Visitor Pattern

Anonymous said...

It is not visitor you use here but strategy.

Anonymous said...

Re: "It is not visitor you use here but strategy."

You're right that it's strategy, but the presence of the double-dispatch makes this a visitor as well (when feedback calls back visitor).