5 PHP Antipatterns That Will DESTROY Your Code (And How They Break SOLID Principles)

I'm a big fan of SOLID principles. They have helped me write clean, maintainable and scalable code over the years. But, I've seen them broken many times, and a few times I was instructed by lead developers to break them myself!

This brings me to the subject of antipatterns. They seem like a good idea at the time, even very clever, but ultimately, they lead to badly written code that's neither clear nor maintainable. This can cause technical debt that isn't immediately obvious, but can cause headaches down the line.

Let's see how antipatterns can break each of the SOLID principles, with examples written in PHP.

Single Responsibility Principle

A class should do only one thing. However, here we have The God Object antipattern that does a few unrelated things, thus breaking this principle:

class UserManager {
    public function createUser($userData) {
        // create user
    }

    public function sendWelcomeEmail($user) {
        // send email
    }

    public function backupUserData() {
        // backup logic
    }
}        

Solution: just break it up into different service classes, each of which does just one thing.

Open-Closed Principle

A class should be open for extension, but closed for modification. In other words, once you create a class, you should not have to modify it's behaviour - instead, you can create a child class that extends the functionality.

Here's the antipattern that breaks this principle:

class PaymentProcessor {
    public function pay($type, $amount) {
        if ($type === 'paypal') {
            // PayPal logic
        } elseif ($type === 'stripe') {
            // Stripe logic
        }
    }
}        

So, what happens if you need to add another payment type, e.g. "square"? You have to modify the if statement in your class.

How to do this properly? Well, create an interface that has a pay method and then a separate class for each payment type, where such a class implements your interface.

Liskov Substitution Principle

A while ago, I wrote an article about this often misunderstood and ignored principle. It basically says that if we can use a parent class for some functionality, then we should be able to use a class that extends the parent, without causing errors.

Here's the antipattern that breaks this principle: we have a parent class called Bird and it has a method called fly().

class Bird {
    public function fly() { /*...*/ }
}

class Sparrow extends Bird {
    public function fly() {/*...*/}
}

class Ostrich extends Bird {
    public function fly() {
        throw new Exception("Ostriches can't fly!");
    }
}        

Now, a sparrow can obviously fly, but can an ostrich? No. So, we have a problem, because we cannot write a program where we expect every bird to fly. LSP is broken in this instance.

How do we remedy this? Maybe with something like this code snippet:

abstract class Bird { }

interface Flyable {
    public function fly();
}

class Sparrow extends Bird implements Flyable {
    public function fly() { /*...*/ }
}

class Ostrich extends Bird {
    // Does not implement Flyable
}        

Interface Segregation Principle

Classes should not depend on interfaces they don't need.

Here's an example of the antipattern that breaks this principle:

interface Worker {
    public function work();
    public function eat();
}

class Robot implements Worker {
    public function work() { /*...*/ }
    public function eat() {
        throw new Exception("Robots don't eat!");
    }
}        

A robot may be a worker, but it doesn't eat, so it should not depend on the Worker interface - it makes no sense.

The solution is to split the functionality into different interfaces and then classes can implement only what they need:

interface Workable {
    public function work();
}

interface Eatable {
    public function eat();
}

class Human implements Workable, Eatable {
    public function work() { /*...*/ }
    public function eat() { /*...*/ }
}

class Robot implements Workable {
    public function work() { /*...*/ }
}        

Dependency Inversion Principle

High level classes should not depend on the low level ones. Both should depend on abstractions.

This antipattern tightly couples the UserRepository class to the MySQLDatabase class. This breaks the principle because if we want to use something other than MySQL, we'd need to rewrite our repository class:

class MySQLDatabase {
    public function connect() { /*...*/ }
}

class UserRepository {
    private $db;

    public function __construct() {
        $this->db = new MySQLDatabase(); // tightly coupled
    }
}        

To remedy, we'd have a general DatabaseConnection interface and inject that into our repository class. Then, it doesn't really matter what kind of DB connection we have, as long as what we include in the constructor of the UserRepository follows the DatabaseConnection contract:

interface DatabaseConnection {
    public function connect();
}

class MySQLDatabase implements DatabaseConnection {
    public function connect() { /*...*/ }
}

class UserRepository {
    private $db;

    public function __construct(DatabaseConnection $db) {
        $this->db = $db;
    }
}        

I know that sometimes we want to solve a problem quickly and that's when the danger of using an antipattern emerges. I'm not saying that we always have to avoid antipatterns, but before you dive into that quick fix, ask yourself if you'd be violating any of the SOLID principles and if it would be worth it in the long run.