Fun with Refactoring

Published on by

Fun with Refactoring image

Refactoring isn't a dirty word, quite the opposite. Refactoring is something that you do when you have leveled up or generally improved. It is impossible to refactor something if you have not improved unless you didn't try the first time, of course!

This tutorial will discuss refactoring, what we could do, and how we might approach a few real-world examples. We will use a real-life project that is open source on GitHub and do a refactoring exercise in stages. The project we will use is Laravel Shift Blueprint, a popular way to bootstrap your Laravel applications.

Let's dive in. I have created a new repository fork to build things in isolation - running GitHub actions in my account so that the core library is not flooded. Now that I have done that, I can clone my fork and start working locally.

The first thing I noticed when opening the project is that composer needs to set a required PHP level. This isn't a huge problem - however, being me, I would put this to the minimum supported PHP version. PHP 8.0 is the only version with enough support to feel comfortable writing this. Let's make our first change.

"require": {
"doctrine/dbal": "^3.3",
"illuminate/console": "^9.0",
"illuminate/filesystem": "^9.0",
"illuminate/support": "^9.0",
"laravel-shift/faker-registry": "^0.2.0",
"symfony/yaml": "^6.0"
},

After updating our dependencies and adding the language constraint, it now looks like the following:

"require": {
"php": "^8.0",
"doctrine/dbal": "^3.5",
"illuminate/console": "^9.39",
"illuminate/filesystem": "^9.39",
"illuminate/support": "^v9.39",
"laravel-shift/faker-registry": "^0.2.0",
"symfony/yaml": "^6.1"
},

We need to run tests before and after each change to ensure that the latest change has not broken anything. We either need to revert or refactor the broken part if it has. This is a crucial step in the refactoring process, as you always want to ensure you are refactoring for the betterment of the project - not just because you have a different opinion.

OK (430 tests, 2056 assertions)

So far, so good! Let's now move on and look at the code specifically. I noticed it was not taking advantage of type hinting or return types when opening a random file in the codebase. Also, as we have now set the minimum PHP version, we can update how we use the PHP language. Let's look at a quick example:

class Tree
{
private $tree;
 
public function __construct(array $tree)
{
$this->tree = $tree;
 
$this->registerModels();
}
 
private function registerModels()
{
$this->models = array_merge($this->tree['cache'] ?? [], $this->tree['models'] ?? []);
}
 
// More code was there, but let's simplify
}

Looking at this code, we know we can make some quick wins in refactoring - just to the language level we now want to set at the minimum. First, let us fix that constructor and remove the property.

class Tree
{
public function __construct(
private array $tree,
) {
$this->registerModels();
}
 
// ...
}

Moving over to using constructor property promotion allows us to simplify our code. Let's run our tests.

OK (430 tests, 2056 assertions)

Still good. We made an easy refactor of this code and can move on to the next part. Let's look at the method called in the constructor. We have a dynamic property on the class - which is being dropped in future versions of PHP support. While it may not be a problem right now - we know that in the future, it will be. So we can refactor this in advance.

class Tree
{
public function __construct(
private array $tree,
private array $models = [],
) {
$this->registerModels();
}
 
private function registerModels(): void
{
$this->models = [
...$this->tree['cache'] ?? [],
...$this->tree['models'] ?? []
];
}
 
// ...
}

We have now set the property for the class in the constructor - and refactored the array merge to use array destructuring. This makes our code much easier to read. Let's rerun those tests!

OK (430 tests, 2056 assertions)

Let us look at the following method: a getter to get the controllers from the Tree class. We currently have no return type and need to know what this may contain. Doing a quick look through the code base using "Find Usages" I see that elsewhere we are dynamically setting this to an instance of Controller. Let's have a look at this method:

public function controllers()
{
return $this->tree['controllers'];
}

Firstly, we want to add the return type, which will be an array.

public function controllers(): array
{
return $this->tree['controllers'];
}

So far, so good. All tests are still passing. But is this enough? Should we add a dynamic type setting in our project when we can let our project know about this in one place? Let's update this.

/**
* @return array<int,Controller>
*/
public function controllers(): array
{
return $this->tree['controllers'];
}

We know this will return a Controller, so why not set this type in our docblock as generic so we can make the most of modern PHP? Now to rerun our tests.

OK (430 tests, 2056 assertions)

So far, our refactors have been successful. Let's move on to another example.

Next, let's look at the ServiceProvider for this package. Like the other example, there aren't any return types - so I will quickly get them added and not worry about those examples here. Let's have a look at how the register method could be improved.

public function register(): void
{
$this->mergeConfigFrom(
__DIR__ . '/../config/blueprint.php',
'blueprint'
);
 
File::mixin(new FileMixins());
 
$this->app->bind('command.blueprint.build', fn ($app) => new BuildCommand($app['files'], app(Builder::class)));
$this->app->bind('command.blueprint.erase', fn ($app) => new EraseCommand($app['files']));
$this->app->bind('command.blueprint.trace', fn ($app) => new TraceCommand($app['files'], app(Tracer::class)));
$this->app->bind('command.blueprint.new', fn ($app) => new NewCommand($app['files']));
$this->app->bind('command.blueprint.init', fn ($app) => new InitCommand());
$this->app->bind('command.blueprint.stubs', fn ($app) => new PublishStubsCommand());
 
$this->app->singleton(Blueprint::class, function ($app) {
$blueprint = new Blueprint();
$blueprint->registerLexer(new \Blueprint\Lexers\ConfigLexer($app));
$blueprint->registerLexer(new \Blueprint\Lexers\ModelLexer());
$blueprint->registerLexer(new \Blueprint\Lexers\SeederLexer());
$blueprint->registerLexer(new \Blueprint\Lexers\ControllerLexer(new \Blueprint\Lexers\StatementLexer()));
 
foreach (config('blueprint.generators') as $generator) {
$blueprint->registerGenerator(new $generator($app['files']));
}
 
return $blueprint;
});
 
$this->app->make('events')->listen(CommandFinished::class, function ($event) {
if ($event->command == 'stub:publish') {
$this->app->make(Kernel::class)->queue('blueprint:stubs');
}
});
 
$this->commands([
'command.blueprint.build',
'command.blueprint.erase',
'command.blueprint.trace',
'command.blueprint.new',
'command.blueprint.init',
'command.blueprint.stubs',
]);
}

First up, we are binding quite a bit to the container and using string constants, which is ok - but we could quickly improve this.

$this->app->bind(BuildCommand::class, fn ($app) => new BuildCommand($app['files'], app(Builder::class)));
$this->app->bind(EraseCommand::class, fn ($app) => new EraseCommand($app['files']));
$this->app->bind(TraceCommand::class, fn ($app) => new TraceCommand($app['files'], app(Tracer::class)));
$this->app->bind(NewCommand::class, fn ($app) => new NewCommand($app['files']));
$this->app->bind(InitCommand::class, fn ($app) => new InitCommand());
$this->app->bind(PublishStubsCommand::class, fn ($app) => new PublishStubsCommand());

This is the first step; next, let us add named arguments to make this look cleaner.

$this->app->bind(
abstract: BuildCommand::class,
concrete: fn (Application $app): BuildCommand => new BuildCommand(
filesystem: $app['files'],
builder: $app->make(
abstract: Builder::class,
),
),
);
$this->app->bind(
abstract: EraseCommand::class,
concrete: fn (Application $app): EraseCommand => new EraseCommand(
filesystem: $app['files'],
),
);
$this->app->bind(
abstract: TraceCommand::class,
concrete: fn (Application $app): TraceCommand => new TraceCommand(
filesystem: $app['files'],
tracer: $app->make(
abstract: Tracer::class,
),
),
);
$this->app->bind(
abstract: NewCommand::class,
concrete: fn (Application $app): NewCommand => new NewCommand(
filesystem: $app['files'],
),
);
$this->app->bind(
abstract: InitCommand::class,
concrete: fn (): InitCommand => new InitCommand(),
);
$this->app->bind(
abstract: PublishStubsCommand::class,
concrete: fn () => new PublishStubsCommand(),
);

Things, at least to me, now look a lot cleaner. We have more type hinting added and return types added for clarity. Not everyone will agree with the named arguments, but this makes the code nicer to read.

The purpose of refactoring is to make the code more manageable, performant, or both. What we have done above displays how we can make the code more type-safe and easier to manage moving forward. Blueprint is quite a complicated project. It has a lot of moving parts to make sure it works. Without spending a long time diving into the specifics of the project and its future goals, it is hard to see what improvements could be made.

My typical approach to refactoring is to hit those quick wins first, the supported language level and package version, then type hinting and return types, then move on to modernizing parts of the code. It is crucial to make sure, as you step through it, that you ensure tests are still running.

Steve McDougall photo

Technical writer at Laravel News, Developer Advocate at Treblle. API specialist, veteran PHP/Laravel engineer. YouTube livestreamer.

Cube

Laravel Newsletter

Join 40k+ other developers and never miss out on new tips, tutorials, and more.

Laravel Forge logo

Laravel Forge

Easily create and manage your servers and deploy your Laravel applications in seconds.

Laravel Forge
Tinkerwell logo

Tinkerwell

The must-have code runner for Laravel developers. Tinker with AI, autocompletion and instant feedback on local and production environments.

Tinkerwell
No Compromises logo

No Compromises

Joel and Aaron, the two seasoned devs from the No Compromises podcast, are now available to hire for your Laravel project. ⬧ Flat rate of $7500/mo. ⬧ No lengthy sales process. ⬧ No contracts. ⬧ 100% money back guarantee.

No Compromises
Kirschbaum logo

Kirschbaum

Providing innovation and stability to ensure your web application succeeds.

Kirschbaum
Shift logo

Shift

Running an old Laravel version? Instant, automated Laravel upgrades and code modernization to keep your applications fresh.

Shift
Bacancy logo

Bacancy

Supercharge your project with a seasoned Laravel developer with 4-6 years of experience for just $2500/month. Get 160 hours of dedicated expertise & a risk-free 15-day trial. Schedule a call now!

Bacancy
Lucky Media logo

Lucky Media

Get Lucky Now - the ideal choice for Laravel Development, with over a decade of experience!

Lucky Media
Lunar: Laravel E-Commerce logo

Lunar: Laravel E-Commerce

E-Commerce for Laravel. An open-source package that brings the power of modern headless e-commerce functionality to Laravel.

Lunar: Laravel E-Commerce
LaraJobs logo

LaraJobs

The official Laravel job board

LaraJobs
SaaSykit: Laravel SaaS Starter Kit logo

SaaSykit: Laravel SaaS Starter Kit

SaaSykit is a Laravel SaaS Starter Kit that comes with all features required to run a modern SaaS. Payments, Beautiful Checkout, Admin Panel, User dashboard, Auth, Ready Components, Stats, Blog, Docs and more.

SaaSykit: Laravel SaaS Starter Kit
Rector logo

Rector

Your partner for seamless Laravel upgrades, cutting costs, and accelerating innovation for successful companies

Rector
MongoDB logo

MongoDB

Enhance your PHP applications with the powerful integration of MongoDB and Laravel, empowering developers to build applications with ease and efficiency. Support transactional, search, analytics and mobile use cases while using the familiar Eloquent APIs. Discover how MongoDB's flexible, modern database can transform your Laravel applications.

MongoDB

The latest

View all →
A Resize Plugin for Alpine.js image

A Resize Plugin for Alpine.js

Read article
How to Migrate MySQL from DBngin to Laravel Herd image

How to Migrate MySQL from DBngin to Laravel Herd

Read article
Learn to master Query Scopes in Laravel image

Learn to master Query Scopes in Laravel

Read article
How to Redirect Uppercase URLs to Lowercase with Laravel Middleware image

How to Redirect Uppercase URLs to Lowercase with Laravel Middleware

Read article
PHP 8.4 Alpha 1 is now out! image

PHP 8.4 Alpha 1 is now out!

Read article
Generics Added to Eloquent Builder in Laravel 11.15 image

Generics Added to Eloquent Builder in Laravel 11.15

Read article