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

Visitor that changes the structure of the objects it visits

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

Problem

Brief summary:

The vanilla GoF visitor is great for altering items within a tree of elements, but when the visitor visits an element it can only change the children of that element not the element itself. For example, a visitor altering the DOM of a webpage could search for everything that contains an image and replace it with an ascii art version of that image. However, `s can contains images, s can contain images, paragraphs (
) can contain images. When the visitor is visiting the node for tag itself it cannot change the type of the image node, though you could change the content of the image node - this is just how visitors work. Instead you would have to find everything that could conceivably contain an image and then visit that - and on top of that anytime W3C added another item that could contain an image you would have to update your visitor.

This isn't a perfect example - there a lots of tools for altering webpage DOMs - but hopefully it is an intuitive one.

Sorry - there is a wall of text in the more complete description below. I'm trying to walk the line between brevity and fully explaining everything, but it looks like this post same out quite wordy.

What I have:

I have a class hierarchy that can be represented by the simplified classes below:

Element is the abstract base class for all visitable classes, and has an Accept method.

LeafElementA and LeafElementB are concretes that are extremely simple. They don't do anything, but just represent that there can be different types of leaves.

MultiCompositeElement and BinaryCompositeElement are also concretes that can contain other Elements - either a collection, or a Left and Right (respectively). The Accept method overrides handle recursing to the contained Elements.

I also have some Visitors that visit each of the
Elements`. All vanilla GoF visitor so far...

(I'm not looking for feedback on this class hierarchy, I'm just setting the scene with a simplified version of the

Solution

Review


Firstly, is there a name for this variant of the visitor pattern?

You have made what I like to call the inviting visitor pattern. I'm not sure this pattern is common, but I have seen it before, disguised and usually involves 3 (types of) classes instead of 2, each with their own concern.

  • inviter: let visitee invite visitor



  • visitee: let visitor accept me



  • visitor: visit reference



As in contrast of the visitor pattern as we all know it:

  • visitee: let visitor accept me



  • visitor: visit visitee



Your visitor is also the (self-)inviter. This part is what I would rename if using the augmented visitor pattern. However, your problem is solvable with a regular visitor pattern.

public virtual Element Visit(Element element)
 {
    element = element.Accept(this);
    return element;
 }


public virtual Element Invite(Element element)
 {
    element = element.Accept(this);
    return element;
 }



Secondly, if this is not the case, then is there a way I can do this
better? Doubling up the visit methods seems wasteful.

Since you never override Visit(Element element), I don't see its purpose. If you would have a situation where you could override, consider moving this functionality to another class. You shouldn't let the visitor decide both (A) how the reference can accept the visitor and (B) how to visit the reference, these are two different concerns.

Proposed Solution

Your situation allows for a regular visitor pattern.

Change the flow in the reference:

internal override Element Accept(IVisitor visitor)
{
    // Recursively visit the children
    Left = visitor.Visit(Left);
    Right = visitor.Visit(Right);

    // And non-recursively visit this
    return visitor.VisitNonRecursive(this);
}


internal override Element Accept(IVisitor visitor)
{
    Left = Left.Accept(visitor);
    Right = Right.Accept(visitor);

    return visitor.Visit(this);
}


Change the flow in the visitor and renameVisitNonRecursive to Visit.

public virtual Element Visit(Element element)
{
    element = element.Accept(this);
    return element;
}


public virtual Element Visit(Element element)
{
     if (element is LeafElementA) return Visit(element as LeafElementA);
     if (element is LeafElementB) return Visit(element as LeafElementB);
     // ..
     return element;
}

Code Snippets

public virtual Element Visit(Element element)
 {
    element = element.Accept(this);
    return element;
 }
public virtual Element Invite(Element element)
 {
    element = element.Accept(this);
    return element;
 }
internal override Element Accept(IVisitor visitor)
{
    // Recursively visit the children
    Left = visitor.Visit(Left);
    Right = visitor.Visit(Right);

    // And non-recursively visit this
    return visitor.VisitNonRecursive(this);
}
internal override Element Accept(IVisitor visitor)
{
    Left = Left.Accept(visitor);
    Right = Right.Accept(visitor);

    return visitor.Visit(this);
}
public virtual Element Visit(Element element)
{
    element = element.Accept(this);
    return element;
}

Context

StackExchange Code Review Q#144625, answer score: 3

Revisions (0)

No revisions yet.