Skip to content

instanceof checks in ExpressionVisitor #294

Open
@rela589n

Description

Hello. I am wondering why does \Doctrine\Common\Collections\Expr\ExpressionVisitor::dispatch restricts $expr to be instance of predefined classes.

Doesn't it violate key principles of visitor pattern which leverage polymorphism and double dispatch in order to avoid such hard-coding of instanceof checks.

public function dispatch(Expression $expr)
{
    switch (true) {
        case $expr instanceof Comparison:
            return $this->walkComparison($expr);

        case $expr instanceof Value:
            return $this->walkValue($expr);

        case $expr instanceof CompositeExpression:
            return $this->walkCompositeExpression($expr);

        default:
            throw new RuntimeException('Unknown Expression ' . get_class($expr));
    }
}

CMIIW, foregoing code can be replaced with single line like this:

public function dispatch(Expression $expr)
{
    return $expr->visit($this);
}

Moreover, this method seems useless to me as we can (and should) call visit directly in the client code. If not, then what is the reason of Expression interface and implementations of visit method if those are not used at all?

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions