One of the benefits of an object oriented programming language is that functionality can be built into the object hierarchy in layers, deferring some details to a descendant while providing common functionality in a base class and avoiding repetition. However, when a group of classes all need to implement nearly the same method but with minor, class specific differences (more than just a constant), a curious anti-pattern tends to arise. In this situation, the base class implementation contains different behaviors based on the type identification of the true object even though we desire to push those implementations to the descendants.

In the Star Wars Combine's (SWC) codebase there are several examples of this, but today we'll use one from the actions system to illustrate the point. In this game, there are many different forms of travel, each of which works mostly the same way. Each works in steps, where the system establishes a deadline for the next travel update and then changes an entity's position after it expires. There are also common behaviors that need to be included, such as awarding experience points and sending events when travel expires.

The core functionality is captured in the abstract class StepTravelAction, which defines several key methods:

abstract class StepTravelAction extends BaseAction {

    public function setup(Entity $leader, $x, $y) {
        // ...
    }

    public function timerInterrupt(TimerInterrupt $payload) {
        // ...
    }

    public function abort() {
        // ...
    }

    public function finishTravelling($reason = null) {
        $leader = $this->leader;
        $travelType = $this->getTravelType();
        $party = $this->getParty();

        XPUtil::giveTravelXP($this->leader, $party, $this->XP, 'Finished ' . $this->getEventText());
        RewardUtil::testUniqueRewards($this->leader, enumRewardType::GROUND_TRAVEL);

        foreach ($this->getParty() as $member) {
            $member->travelID = null;
        }
        EntityLocationUtil::saveEntityLocation($leader);

        $this->delete();

        if ($reason == null) {
            $reason = TravelEndInterrupt::FINISH_NORMAL;
        }

        InterruptUtil::fire($leader, new TravelEndInterrupt($leader, $travelType, $reason));
    }

    abstract public function setupTravel();
    abstract public function applyStep(Entity $leader, Point $next);
    abstract public function getTravelType();
    // ...

}

Although the function bodies for some functions have been removed in the interest of space, we can see the core functionality, like aborting or finishing travel, is handled here in the base class. The concrete, travel type-specific details of things, like updating the entity's position, are farmed out to the derived classes.

However, at some point, it was decided that ground travel was awarding too much experience and could be abused by players (mostly to power level NPCs who can be set to patrol a location indefinitely, which takes months otherwise). Experience is awarded inside finishTravelling() as we can see, which is "common" to all of the travel action classes. At this point, there are several options for modifying the implementation, and the "simplest" in a language with dynamic type identification produces a design antipattern.

To reduce the XP awarded in ground travel only, an earlier programmer elected to add three lines to StepTravelAction::finishTravelling(), quickly resolving the issue:

    public function finishTravelling($reason = null) {
        // ...

        if ($this instanceof GroundTravelAction) {
            this->XP / 5);
        }

        // ...
    }

This ignores the benefits of object inheritance, produces less elegant code, and reduces the sustainability of the code in the future. Behavior specific to GroundTravelAction is now no longer contained within GroundTravelAction itself, so if we wanted to further modify the XP for this case, a lot of code walking would be needed to figure out where to do it. If multiple such exceptions are added, you might as well not have support for polymorphism at all and do everything in a single struct that stores type IDs and uses switch statements, taking us back to the early 80s. Exceptions like this were added to several other methods (abort(), etc.) for the same change as well.

The correct approach here is to refactor this method into three different components:

1. a concrete version of finishTravelling that follows the original implementation
2. a virtual method implemented in StepTravelAction that simply forwards to the concrete implementation
3. a virtual method in the descending class that layers additional functionality (such as changing the XP to be awarded) and then calls the concrete version from its parent

We need all three components because the default implementation is correct in most cases, so it should be available as the default behavior, but when we want to modify it we still need to have the original version available to avoid copying and pasting it into the derived class. I think it might be even worse if someone were to simply use a virtual method and copy it entirely into the derived class in order to add three lines.

For completeness, a preferable implementation would look like this and preserve the same behavior for all other derived classes:

abstract class StepTravelAction extends BaseAction {

    protected function _finishTravelling($reason = null) {
        $leader = $this->leader;
        $travelType = $this->getTravelType();
        $party = $this->getParty();

        XPUtil::giveTravelXP($this->leader, $party, $this->XP, 'Finished ' . $this->getEventText());
        RewardUtil::testUniqueRewards($this->leader, enumRewardType::GROUND_TRAVEL);

        foreach ($this->getParty() as $member) {
            $member->travelID = null;
        }
        EntityLocationUtil::saveEntityLocation($leader);

        $this->delete();

        if ($reason == null) {
            $reason = TravelEndInterrupt::FINISH_NORMAL;
        }

        InterruptUtil::fire($leader, new TravelEndInterrupt($leader, $travelType, $reason));
    }

    public function finishTravelling($reason = null) {
        $this->_finishTravelling($reason);
    }

}

class GroundTravelAction extends StepTravelAction {

    public function finishTravelling($reason = null) {
        $this->XP = ceil($this->XP / 5);
        $this->_finishTravelling($reason);
    }

}

The reason I want to highlight this is that the problem arose despite the developer having a familiarity with inheritance and deferring implementation details to derived classes where meaningful. There are abstract methods here and they are used to plug into common functionality implemented within StepTravelAction, but while targeting a change in behavior it is easy to lose sight of the overall design and simply insert a change where the implementation is visible. This kind of polymorphism antipattern is one of the most common implementation issues in SWC, likely due to the availability of the instanceof operator in PHP. In C++ it is a lot harder to do this (although RTTI does exist, I never use it), and people are often forced to learn the correct approach as a result.