Interested how Laravel works under the hood? → Check my Laravel Core Adventures Course

Refactoring PHP

#php

I've been programming in PHP now for almost ten years, and if there is one thing I learned over this period, it's that readability and simplicity are the keys for maintainable and sustainable code. Every first attempt to write code should be about making it work. Only after that, you should take some time to refactor. This is when I aim for readability and simplicity. Today I see refactoring as one of my main skills. In this post, I share with you my refactoring practices for PHP.

Refactoring is the process of modifying and restructuring code without changing its functionality. When I first heard about it, I was like: Why would anyone do that? It took some years until I fully understood the concept and that the working code is not always good. So with refactoring, you can make working code better, and what better means is something developers will argue for hours. In the end, you have to define it for yourself.

In the following examples, I will show you how I define better code. Are you just looking for a specific strategy? Please use the anchor links:

#1 - Be Expressive

This might be an easy tip, but writing expressive code can improve it a lot. Always make your code self-explaining so that you, your future self or any other developers who stumble over your code knows what is going on.

But then developers also say naming is one of the hardest things in programming. This is one reason why this is not as easy as it sounds. 🤷‍

Note: Make sure to hit the "Show/Hide Notes" buttons on the code examples to get details about why we needed to change the code.

Example #1 - Naming

$status = $user->status('pending');

// ❌ It is unclear what this method is doing with the provided string
// ❌ Are we setting the status? Are we checking the status?
$status = $user->status('pending');

$isUserPending = $user->isStatus('pending');

// ✅ By adding "is" to the names we make our intentions more clear
// ✅ We are checking if the status of the user equals the given string
// ✅ Also the new variable name lets us assume it will be a boolean
$isUserPending = $user->isStatus('pending');

Example #2 - Naming

return $factory->getTargetClass();

// ❌ What do we get? The class name? Fully qualified name? Or the path?
return $factory->getTargetClass();

return $factory->getTargetClassPath();

// ✅ It is the path we are getting back
// ✅ If the user looks for the class name, this is the wrong method
return $factory->getTargetClassPath();

Example #3 - Extracting

public function setCodeExamples(string $exampleBefore, string $exampleAfter)
{
    $this->exampleBefore = file_get_contents(base_path("$exampleBefore.md"));
    $this->exampleAfter = file_get_contents(base_path("$exampleAfter.md"));
}

// ❌ Duplicated code (methods "file_get_contents", "base_path" and file extension)
// ❌ At this point we don't care HOW we get the code examples
public function setCodeExamples(string $exampleBefore, string $exampleAfter)
{
    $this->exampleBefore = file_get_contents(base_path("$exampleBefore.md"));
    $this->exampleAfter = file_get_contents(base_path("$exampleAfter.md"));
}

public function setCodeExamples(string $exampleBefore, string $exampleAfter)
{
    $this->exampleBefore = $this->getCodeExample($exampleBefore);
    $this->exampleAfter = $this->getCodeExample($exampleAfter);
}

private function getCodeExample(string $exampleName): string
{
    return file_get_contents(base_path("$exampleName.md"));
}

public function setCodeExamples(string $exampleBefore, string $exampleAfter)
{ 
    // ✅ Our code now tells what we are doing: getting a code example (we do not care how)
    $this->exampleBefore = $this->getCodeExample($exampleBefore);
    $this->exampleAfter = $this->getCodeExample($exampleAfter);
}

// ✅ The new method can be used multiple times now
private function getCodeExample(string $exampleName): string
{
    return file_get_contents(base_path("$exampleName.md"));
}

Example #4 - Extracting

User::whereNotNull('subscribed')->where('status', 'active');

// ❌ Multiple where clauses make it difficult to read
// ❌ What is the purpose?
User::<hljs general>whereNotNull('subscribed')->where('status', 'active')</hljs>;

User::subscribed();

// ✅ This new scope method tells us what is happening
// ✅ If we need more details we check the scope method
// ✅ The scope "subscribed" can be used somewhere else too now
User::subscribed();

Example #5 - Extracting

This is an example of one of my projects from last year. We had a command-line command to import users. The class ImportUsersCommand contains a handle method to perform the tasks.

protected function handle()
{
    $url = $this->option('url') ?: $this->ask('Please provide the URL for the import:');
    
    $importResponse =  $this->http->get($url);

    $bar = $this->output->createProgressBar($importResponse->count());
    $bar->start();

    $this->userRepository->truncate();
    collect($importResponse->results)->each(function (array $attributes) use ($bar) {
        $this->userRepository->create($attributes);
        $bar->advance();
    });

    $bar->finish();
    $this->output->newLine();

    $this->info('Thanks. Users have been imported.');
    
    if($this->option('with-backup')) {
        $this->storage
            ->disk('backups')
            ->put(date('Y-m-d').'-import.json', $response->body());

        $this->info('Backup was stored successfully.');
    }
  
}

protected function handle()
{
    // ❌ The method contains too much code
    $url = $this->option('url') ?: $this->ask('Please provide the URL for the import:');
    
    $importResponse =  $this->http->get($url);
    
    // ❌ The progress bar output is useful for the user but makes our code messy
    $bar = $this->output->createProgressBar($importResponse->count());
    $bar->start();
    
    $this->userRepository->truncate();
    collect($importResponse->results)->each(function (array $attributes) use ($bar) {
        $this->userRepository->create($attributes);
        $bar->advance();
    });
    
    // ❌ It is difficult to tell which actions are being taken here
    $bar->finish();
    $this->output->newLine();

    $this->info('Thanks. Users have been imported.');
    
    if($this->option('with-backup')) {
        $this->storage
            ->disk('backups')
            ->put(date('Y-m-d').'-import.json', $response->body());

        $this->info('Backup was stored successfully.');
    }
  
}

protected function handle()
{
    $url = $this->option('url') ?: $this->ask('Please provide the URL for the import:');
    
    $importResponse =  $this->http->get($url);
    
    $this->importUsers($importResponse->results);
    
    $this->saveBackupIfAsked($importResponse);
}

protected function importUsers($userData): void
{
    $bar = $this->output->createProgressBar(count($userData));
    $bar->start();

    $this->userRepository->truncate();
    collect($userData)->each(function (array $attributes) use ($bar) {
        $this->userRepository->create($attributes);
        $bar->advance();
    });

    $bar->finish();
    $this->output->newLine();

    $this->info('Thanks. Users have been imported.');
}

protected function saveBackupIfAsked(Response $response): void
{
    if($this->option('with-backup')) {
        $this->storage
            ->disk('backups')
            ->put(date('Y-m-d').'-import.json', $response->body());

        $this->info('Backup was stored successfully.');
    }
}

protected function handle(): void
{
    // ✅ The handle method is what you check first when visiting this class
    // ✅ It is now easy to get an overview of what is happening
    $url = $this->option('url') ?: $this->ask('Please provide the URL for the import:');
    
    $importResponse =  $this->http->get($url);
    
    $this->importUsers($importResponse->results);
    
    $this->saveBackupIfAsked($importResponse);
}

// ✅ If you need more details, you can check the dedicated methods
protected function importUsers($userData): void
{
    $bar = $this->output->createProgressBar(count($userData));
    $bar->start();

    $this->userRepository->truncate();
    collect($userData)->each(function (array $attributes) use ($bar) {
        $this->userRepository->create($attributes);
        $bar->advance();
    });

    $bar->finish();
    $this->output->newLine();

    $this->info('Thanks. Users have been imported.');
}

// ✅ Don't be afraid of more lines of code
// ✅ In this example it makes sense to clean up our main "handle" method
protected function saveBackupIfAsked(Response $response): void
{
    if($this->option('with-backup')) {
        $this->storage
            ->disk('backups')
            ->put(date('Y-m-d').'-import.json', $response->body());

        $this->info('Backup was stored successfully.');
    }
}

#2 - Return Early

The concept of early returns refers to a practice where we try to avoid nesting by breaking a structure down to specific cases. In return, we will get a more linear code, which is much easier to read and grasp. Every case is separated and good to follow. Don't be afraid of using multiple return statements.

Example #1

public function calculateScore(User $user): int
{
    if ($user->inactive) {
        $score = 0;
    } else {
        if ($user->hasBonus) {
            $score = $user->score + $this->bonus;
        } else {
            $score = $user->score;
        }
    }

    return $score;
}

public function calculateScore(User $user): int
{
    if ($user->inactive) {
        $score = 0;
    } else {
        // ❌ What was "if" again?
        if ($user->hasBonus) {
            $score = $user->score + $this->bonus;
        } else {
            // ❌ Your 👀 are constantly moving due to the different idention levels
            $score = $user->score;
        }
    }

    return $score;
}

public function calculateScore(User $user): int
{
    if ($user->inactive) {
        return 0;
    }

    if ($user->hasBonus) {
        return $user->score + $this->bonus;
    }

    return $user->score;
}

public function calculateScore(User $user): int
{
    // ✅ Edge-cases are checked first
    if ($user->inactive) {
        return 0;
    }

    // ✅ Every case has its own section which makes them easy to follow step by step
    if ($user->hasBonus) {
        return $user->score + $this->bonus;
    }

    return $user->score;
}

Example #2

public function sendInvoice(Invoice $invoice): void
{
    if($user->notificationChannel === 'Slack')
    {
        $this->notifier->slack($invoice);
    } else {
        $this->notifier->email($invoice);
    }
}

public function sendInvoice(Invoice $invoice): void
{
    if($user->notificationChannel === 'Slack')
    {
        $this->notifier->slack($invoice);
    } else {
        // ❌ Even a simple ELSE effects the readability of your code
        $this->notifier->email($invoice);
    }
}

public function sendInvoice(Invoice $invoice): bool
{
    if($user->notificationChannel === 'Slack')
    {
        return $this->notifier->slack($invoice);
    }

    return $this->notifier->email($invoice);
}

public function sendInvoice(Invoice $invoice): bool
{
    // ✅ Every condition is easy to read
    if($user->notificationChannel === 'Slack')
    {
        return $this->notifier->slack($invoice);
    }

    // ✅ No more thinking about what ELSE refers to
    return $this->notifier->email($invoice);
}
Note: You sometimes also hear the term "guard-clauses" which is what you can achieve with early returns. You are guarding your method from special conditions.

#3 - Refactor To Collections

In PHP, we work a lot with arrays of different data. The available features to handle and transform those arrays are quite limited in PHP and don't provide a good experience. (array_walk, usort, etc.)

To tackle this problem, there is this concept of Collection classes, which help you handle arrays. Most known is the implementation in Laravel, where a collection class provides you with dozens of useful features to work with arrays.

Note: For the following examples, I will use Laravel's collect() helper, but the approach is very similar with other available collection classes from other frameworks or libraries.

Example #1

$score = 0;

foreach($this->playedGames as $game) {
    $score += $game->score;
}

return $score;

// ❌ Here we have a temporary variable
$score = 0;

// ❌ It is ok to use a loop, but it could be more readable
foreach($this->playedGames as $game) {
    $score += $game->score;
}

return $score;

return collect($this->playedGames)
    ->sum('score');

// ✅ The collection is an object with methods
// ✅ The sum method makes it more expressive
return collect($this->playedGames)
    ->sum('score');

Example #2

$users = [
    [ 'id' => 801, 'name' => 'Peter', 'score' => 505, 'active' => true],
    [ 'id' => 844, 'name' => 'Mary', 'score' => 704, 'active' => true],
    [ 'id' => 542, 'name' => 'Norman', 'score' => 104, 'active' => false],
];

// Requested Result: only active users, sorted by score ["Mary(704)","Peter(505)"]

$users = array_filter($users, fn ($user) => $user['active']);

usort($users, fn($a, $b) => $a['score'] < $b['score']);

$userHighScoreTitles = array_map(fn($user) => $user['name'] . '(' . $user['score'] . ')', $users);

return $userHighScoreTitles;

$users = [
    [ 'id' => 801, 'name' => 'Peter', 'score' => 505, 'active' => true],
    [ 'id' => 844, 'name' => 'Mary', 'score' => 704, 'active' => true],
    [ 'id' => 542, 'name' => 'Norman', 'score' => 104, 'active' => false],
];

// Requested Result: only active users, sorted by score ["Mary(704)","Peter(505)"]

$users = array_filter($users, fn ($user) => $user['active']);

// ❌ Which is "usort" again? How does the condition work?
usort($users, fn($a, $b) => $a['score'] < $b['score']);

// ❌ All the transformations are separated, still they all are about users
$userHighScoreTitles = array_map(fn($user) => $user['name'] . '(' . $user['score'] . ')', $users);

return $userHighScoreTitles;

$users = [
    [ 'id' => 801, 'name' => 'Peter', 'score' => 505, 'active' => true],
    [ 'id' => 844, 'name' => 'Mary', 'score' => 704, 'active' => true],
    [ 'id' => 542, 'name' => 'Norman', 'score' => 104, 'active' => false],
];

// Requested Result: only active users, sorted by score ["Mary(704)","Peter(505)"]

return collect($users)
  ->filter(fn($user) => $user['active'])
  ->sortBy('score')
  ->map(fn($user) => "{$user['name']} ({$user['score']})"
  ->values()
  ->toArray();

$users = [
    [ 'id' => 801, 'name' => 'Peter', 'score' => 505, 'active' => true],
    [ 'id' => 844, 'name' => 'Mary', 'score' => 704, 'active' => true],
    [ 'id' => 542, 'name' => 'Norman', 'score' => 104, 'active' => false],
];

// Requested Result: only active users, sorted by score ["Mary(704)","Peter(505)"]

// ✅ We are passing users only once
return collect($users)
    // ✅ We are piping them through all methods
  ->filter(fn($user) => $user['active'])
  ->sortBy('score')
  ->map(fn($user) => "{$user['name']} ({$user['score']})"
  ->values()
    // ✅ At the end, we return an array
  ->toArray();
Note: Adam Wathan wrote a book about how to use collections to never write a loop again which I can highly recommend.

#4 - Consistency

Every line of code adds a little amount of visual noise. The more code, the more difficult it gets to read. This is why it is essential to set rules. Keeping similar things consistent will help you recognize code and patterns. It will result in less noise and more readable code.

Example #1

class UserController 
{
    public function find($userId)
    {
    
    }
}

class InvoicesController 
{
    public function find($user_id) {
    
    }
}

class UserController 
{
    // ❌ Decide how to name variables (camelCase, snake_case, etc.) Don't mix!
    public function find($userId)
    {
    
    }
}

// ❌ Choose between singular or plural for all controllers and stick to it
class InvoicesController 
{
    // ❌ Changes in style, like position of braces, make your code hard to read
    public function find($user_id) {
    
    }
}

class UserController 
{
    public function find($userId)
    {
    
    }
}

class InvoiceController 
{
    public function find($userId)
    {
    
    }
}

class UserController 
{
    // ✅ camelCase Naming for all variables
    public function find($userId)
    {
    
    }
}

// ✅ Controller names consistent (singular in this case)
class InvoiceController 
{
    // ✅ Consistent position of braces (format) makes code more readable
    public function find($userId)
    {
    
    }
}

Example #2

class PdfExporter
{
    public function handle(Collection $items): void
    {
        // export items...
    }
}

class CsvExporter
{
    public function export(Collection $items): void
    {
        // export items...
    }
}

$pdfExport->handle();
$csvExporter->export();

class PdfExporter
{
    // ❌ "handle" and "export" are similar methods with different names
    public function handle(Collection $items): void
    {
        // export items...
    }
}

class CsvExporter
{
    public function export(Collection $items): void
    {
        // export items...
    }
}

// ❌ While using them you will question if they even perform similar tasks
// ❌ I bet you will look up the classes to make sure
$pdfExport->handle();
$csvExporter->export();

interface Exporter
{
    public function export(Collection $items): void;
}

class PdfExporter implements Exporter
{
    public function export(Collection $items): void
    {
        // export items...
    }
}

class CsvExporter implements Exporter
{
    public function export(Collection $items): void
    {
        // export items...
    }
}

$pdfExport->export();
$csvExporter->export();

// ✅ An interface can help with consistency by providing common rules
interface Exporter
{
    public function export(Collection $items): void;
}

class PdfExporter implements Exporter
{
    public function export(Collection $items): void
    {
        // export items...
    }
}

class CsvExporter implements Exporter
{
    public function export(Collection $items): void
    {
        // export items...
    }
}

// ✅ Same method names for similar tasks will make it easier to read
// ✅ I'm pretty sure, without taking a look at the classes, they both export data
$pdfExport->export();
$csvExporter->export();

Refactoring ❤️ Tests

I already mentioned that refactoring doesn't change the functionality of your code. This comes handy when running tests, because they should work after refactoring too. This is why I only start to refactor my code, when there are tests. They will assure that I don't unintentionally change the behaviour of my code. So don't forget to write tests or even go TDD.

Conclusion

This is how I refactor PHP. In the end, you and your team have to decide how you want your code to be. Just make sure to define it, write it down, and save enough time to make it happen after your code is working. I started with my main principles and I plan to cover more topics soon. If you have refactoring tips you'd like to share, just ping me on Twitter.

Do you enjoy my posts?

Sign up for my newsletter to receive updates on my latest content.

You will receive monthly updates on my latest articles and products. I do care about the protection of your data. Read my Privacy Policy.