Visitor pattern - Role interface

[JindrichPilar]

Shouldn't the role interface define accept method, to be sure any Role class can accept RoleVisitor?

Maybe rather than interface use an abstract class to evade repeating accept method code in every subclass.

Or did I get it all wrong?

https://github.com/domnikl/DesignPatternsPHP/blob/master/Visitor/Visitor.php#L17

[Trismegiste]

You're absolutely right, I concur. This pattern is incomplete : accept() method is a mandatory part of visitor pattern.

Since PHP can't use polymorphism based on type-hint, another missing part is a list of visitXXX methods for each concrete elements. To simulate double-dispatch, one could use a get_called_class in "accept()" method.

A complete example for java http://en.wikipedia.org/wiki/Visitor_pattern#Diagram

to simulate type-hint polymorphism :

public function accept(RoleVisitor $visitor)
{
    call_user_func(array($visitor, 'visit' . get_called_class()), $this)
}

that's the idea. Warning, this code fails with namespaced classname... More there's no check or whatsoever.

[JindrichPilar]

Wouldn't be better to pass the class name as parameter to visit method and let the Visitor decide what to do?
Calling visitXXX will force Visitor to know all Role subclasses.

public function accept(RoleVisitor $visitor)
{
    call_user_func(array($visitor, 'visit'), $this,  get_called_class());
}


public function visit(Role $role, $className) 
{
    switch($className) {
       case: 'AdminRole':
           // for more complicated task call another method
           echo "Role: <b>" . $role->getName() . "</b>;
           break;
       default:
            echo "Role: " . $role->getName();
            break;
    }
}

[Trismegiste]

Sorry but I don't think so.

For SRP concern, Visitor doesn't have to "decide" what to do regarding the real type of object. It knows what to extract from one visited object but no more. It's not a problem if Role knows all methods of RoleVisitor, it is designed like this (to show that Liskov Substitution Principle is "handled with care" ).

Therefore, in C++ & Java, a subclass of Car, for example, will be visited by visit(Car car) not by a default implementation for an abstract Element.

And since PHP does not use type-hint polymoprhism for methods, there is always the workaround for a subclass of Car to call visitCar() anyway by overriding accept() method.

Furthermore, this kind of big switch you propose tends to be un-maintainable for future changes.

If you're not comfortable with the dynamic call (that's understandable), you can simply let the visitee decide which method to call like this example does :

http://sourcemaking.com/design_patterns/visitor/php#

Anyway, with good check and exception, I think dynamic calls will run smoothly.

I'm not sure if I'm clear, it's difficult to explain this problem. If you give me some times (one or two days) I can show you the implementation I see.

[Trismegiste]

I've just made a PR between two beach sessions ☀️

(I'm currently now in Caribbean Sea) 🍸

[JindrichPilar]

Thanks, your code shows it really nicely.

Have a nice holiday