Don't knock. You are accepted
TL;DR: Avoid combining the Visitor pattern with instanceof checks.
Problems 😔
- Open/Closed principle violation
- Tight Coupling
- Maintainability
- Code duplication
- Poor readability
- Brittle design
- IFs
Solutions 😃
- Implement the Visitor pattern correctly.
- Avoid instanceof checks.
- Favor polymorphism.
- Encapsulate behavior.
Context 💬
When you use the Visitor pattern, you aim to separate algorithms from the objects they operate on.
Combining it with instanceof checks breaks this separation, leading to a fragile and hard-to-maintain design.
Sample Code 📖
Wrong 🚫
<?php
class SpaceObjectVisitor {
public function visit(SpaceObject $object) {
if ($object instanceof NeutronStar) {
$this->visitNeutronStar($object);
} elseif ($object instanceof Magnetar) {
$this->visitMagnetar($object);
} elseif ($object instanceof BlackHole) {
$this->visitBlackHole($object);
}
// Not closed for modification
}
private function visitNeutronStar(NeutronStar $star) {
// Handle neutron star observation
}
private function visitMagnetar(Magnetar $magnetar) {
// Handle magnetar observation
}
private function visitBlackHole(BlackHole $blackHole) {
// Handle black hole observation
}
}
Right 👉
<?php
interface SpaceObject {
public function accept(SpaceObjectVisitor $visitor): void;
}
class NeutronStar implements SpaceObject {
public function accept(SpaceObjectVisitor $visitor): void {
$visitor->visitNeutronStar($this);
}
}
class Magnetar implements SpaceObject {
public function accept(SpaceObjectVisitor $visitor): void {
$visitor->visitMagnetar($this);
}
}
class BlackHole implements SpaceObject {
public function accept(SpaceObjectVisitor $visitor) {
$visitor->visitBlackHole($this);
}
}
class SpaceObjectVisitor {
// Open for extension
// You can also convert it into an interface
public function visitNeutronStar(NeutronStar $star): void {
// Handle neutron star
}
public function visitMagnetar(Magnetar $magnetar): void {
// Handle magnetar
}
public function visitBlackHole(BlackHole $blackHole): void {
// Handle black hole
}
}
Detection 🔍
You can detect this smell by looking for instanceof checks within the Visitor pattern.
Automated tools can flag these checks, and code reviews can help identify them.
[X] Manual
Tags 🏷️
- IFs
Level 🔋
[X] Intermediate
Why the Bijection Is Important 🗺️
When you model real-world objects, you should maintain a one-to-one correspondence between the real-world entities and your code.
Breaking this correspondence using instanceof checks (which do not exist on the model but the metamodel) leads to a mismatch between the real world and your program.
AI Generation 🤖
AI generators might create this smell if they are not properly guided.
AI Detection 🥃
AI can help fix this smell by suggesting refactoring steps to remove instanceof checks and use polymorphism instead.
Try Them! 🛠
Remember: AI Assistants make lots of mistakes
| Without Proper Instructions | With Specific Instructions |
|---|---|
| ChatGPT | ChatGPT |
| Claude | Claude |
| Perplexity | Perplexity |
| Copilot | Copilot |
| Gemini | Gemini |
| DeepSeek | DeepSeek |
| Meta AI | Meta AI |
Conclusion ✔️
When you use the Visitor pattern, avoid combining it with iskindOf checks.
Using the accept() method, the visitor class doesn't need to explicitly check the object type.
Each object will call the appropriate visitor method honoring the Open/Closed principle.
Favor polymorphism and encapsulation to keep your code clean and maintainable.
Relations 👩❤️💋👨
More Information 📕
Open/Closed principle on Wikipedia
Disclaimer 📘
Code Smells are my opinion.
Credits 🙏
The Visitor pattern is a way to add new operations to a set of objects without changing their classes.
Erich Gamma
This article is part of the CodeSmell Series.