Detecting SOLID Violations in PHP

Identifying a Single Responsibility Principle (SRP) Violation

Let's start with the Single Responsibility Principle, which states that a class should have one, and only one, reason to change. Violations of this principle often manifest as "God Classes" that try to do too much.

Consider the following UserManager class:

class UserManager {
    public function addUser(string $username, string $email) {
        // 1. Validate the user data
        if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
            // error handling...
            return false;
        }

        // 2. Save the user to the database
        $user = new User();
        $user->username = $username;
        $user->email = $email;
        $user->save();
        
        // 3. Log the action
        Log::info("User {$username} created.");

        // 4. Send a welcome email
        Mail::to($email)->send(new WelcomeEmail($user));

        return true;
    }

    // ... other user management methods
}

Take a moment to analyze this addUser method. What are the distinct jobs it's performing? If you were to list the reasons this class might need to be modified in the future, how many would you come up with?

This class clearly violates SRP because it has accumulated multiple responsibilities. We can identify at least four distinct reasons for change:

1. User validation logic changes: If we need to add more complex validation rules.
2. Database schema changes: If the User model or the way we persist it changes.
3. Logging format changes: If we decide to use a different logging channel or message format.
4. Email system changes: If we change the content of the welcome email or the mail service itself.

Each of these is a separate concern. A change in the email template should not require a developer to touch the UserManager class, which also contains database logic. This tight coupling makes the class fragile and harder to test.

The diagram below illustrates this exact problem and the refactored solution. The "Bad Design" shows a single class juggling multiple responsibilities, while the "Good Design" separates these concerns into specialized classes.

Identifying an Open/Closed Principle (OCP) Violation

Next is the Open/Closed Principle: software entities should be open for extension but closed for modification. A common red flag for an OCP violation is a switch statement or a series of if/elseif blocks that handle different types of an object.

Examine this PaymentController:

class PaymentController
{
    public function processPayment(Request $request)
    {
        $paymentMethod = $request->input('payment_method');

        switch ($paymentMethod) {
            case 'paypal':
                $this->processPayPalPayment($request);
                break;
            case 'stripe':
                $this->processStripePayment($request);
                break;
            // What happens when we add a 'braintree' option?
            default:
                // Handle unsupported payment method
                break;
        }
    }

    private function processPayPalPayment(Request $request) { /* ... */ }
    private function processStripePayment(Request $request) { /* ... */ }
}

If your product manager asks you to add support for a new payment gateway, say Braintree, what specific changes would you need to make to this file?

To add Braintree support, you would have to modify the PaymentController class. You would need to add another case 'braintree': to the switch statement and a new processBraintreePayment() method. This directly violates the principle of being "closed for modification." Every new payment method requires altering existing, and likely stable, code, which introduces the risk of breaking what already works. A design that adheres to OCP would allow you to add new payment processors by adding new classes, not by changing existing ones.

Identifying a Liskov Substitution Principle (LSP) Violation

The Liskov Substitution Principle demands that subclasses must be substitutable for their base types without altering the correctness of the program. In essence, a child class must honor the contract of its parent.

Consider this class hierarchy for vehicles:

class Vehicle {
    public function startEngine() {
        echo "Engine started!";
    }
}

class Car extends Vehicle {
    public function startEngine() {
        echo "Car engine started!";
    }
}

class Bicycle extends Vehicle {
    public function startEngine() {
        // Bicycles don't have engines, so this violates LSP
        throw new \Exception("Bicycles don't have engines!");
    }
}

// Client code
function startAVehicle(Vehicle $vehicle) {
    try {
        $vehicle->startEngine();
    } catch (\Exception $e) {
        // The client code is now forced to handle an unexpected exception
        echo "Error: " . $e->getMessage();
    }
}

Imagine the startAVehicle function is part of your application. What assumption does it make about any Vehicle object it receives? How does the Bicycle class break that assumption?

The client function startAVehicle assumes that any object of type Vehicle can have its startEngine() method called without throwing an exception. The Car subclass honors this contract. However, the Bicycle subclass violates it by throwing an exception.

This means a Bicycle object is not a valid substitute for a Vehicle object everywhere. The client code, which should ideally only care about the Vehicle abstraction, is now forced to be aware of the specific behavior of the Bicycle subclass (e.g., by adding a try-catch block it didn't need before). This is a classic LSP violation. A better design would not have Bicycle extend a class that includes an startEngine method.

Identifying an Interface Segregation Principle (ISP) Violation

The Interface Segregation Principle advises that clients should not be forced to depend on interfaces they do not use. This principle targets "fat" interfaces.

Look at the "Bad Design" portion of the following diagram:

In this bad design, we have a single Animal interface:

interface Animal {
    public function fly();
    public function swim();
}

Now, consider a Fish class implementing this interface:

class Fish implements Animal {
    public function fly() {
        // A fish cannot fly. What do we put here?
        return null; 
        // or throw new \Exception('Not implemented');
    }

    public function swim() {
        echo "Fish is swimming.";
    }
}

What is the problem with the fly() method in the Fish class?

The Fish class is forced to provide an implementation for the fly() method, even though it's completely irrelevant to a fish. This leads to awkward code, such as an empty method, a method that returns null, or one that throws an exception. This is a clear signal that the interface is too "fat."

The interface is trying to serve two different types of clients: flying animals and swimming animals. The solution, as shown in the "Good Design" part of the diagram, is to segregate the interface into smaller, more cohesive ones (Flyable, Swimmable). A class can then implement only the interfaces that are relevant to its capabilities.

Identifying a Dependency Inversion Principle (DIP) Violation

Finally, the Dependency Inversion Principle states that high-level modules should not depend on low-level modules; both should depend on abstractions. A tell-tale sign of a DIP violation is the use of the new keyword to instantiate a concrete, low-level dependency inside a high-level class.

Consider this UserService:

class UserService {
    private MySQLDatabase $database;

    public function __construct() {
        // Violation: Direct dependency on a concrete low-level module
        $this->database = new MySQLDatabase();
    }

    public function getUser(int $id): string {
        return $this->database->getUser($id);
    }
}

class MySQLDatabase {
    public function getUser(int $id): string {
        return "User data from MySQL";
    }
}

The UserService is a high-level module containing business logic. The MySQLDatabase is a low-level module dealing with infrastructure details. What is the coupling issue here? If you wanted to test UserService without a real database, or switch to using PostgreSQL, what would you have to change?

The UserService is tightly coupled to the concrete MySQLDatabase class. You cannot switch to a PostgreSQLDatabase or use a mock FakeDatabase for testing without modifying the source code of UserService. The high-level policy (user service logic) depends directly on the low-level detail (a specific database implementation).

A design that follows DIP would have the UserService depend on a DatabaseInterface abstraction. The concrete MySQLDatabase would then be "injected" from the outside, typically via the constructor, a process you are very familiar with from using Laravel's Service Container.

Conclusion

In this lesson, we've trained our eyes to spot common violations of the SOLID principles. Recognizing these patterns is the first and most critical step toward writing more robust and maintainable code.

Here are the key "code smells" to watch for:

• SRP Violation: A class with methods that handle different, unrelated concerns (e.g., business logic, database persistence, logging, and emailing all in one place).
• OCP Violation: The presence of switch or if/elseif chains that select behavior based on an object's type. This forces modification to add new types.
• LSP Violation: A subclass that throws an exception for a method that the parent class doesn't, or that returns a different type, breaking the parent's contract.
• ISP Violation: Classes that are forced to implement methods with empty or exception-throwing bodies because an interface is too broad.
• DIP Violation: The new keyword creating a concrete low-level dependency (like a database connection or mailer) inside a high-level business logic class.

In our next lesson, we will close the loop. You will be given code samples with these exact types of violations, and your task will be to refactor the code to resolve them, applying the correct principles to create a clean, flexible, and solid design.