A practical guide to refactoring PHP code without breaking production, losing your tests or starting a fight in the pull request
The previous article in this series ended with a line about refactoring without tests being just editing with optimism. A few people asked what refactoring actually means in practice. Not the textbook definition. The actual process. What you do on a Tuesday afternoon when you open a file, wince at what you see and have to decide whether to fix it or move on.
This is that article.
What refactoring is not
Before anything else, let us clear up what the word means because it gets misused constantly.
Refactoring is not rewriting. Rewriting means throwing away existing code and starting over. Refactoring means changing the structure of code without changing what it does. Those are fundamentally different activities with fundamentally different risk profiles.
Refactoring is not bug fixing. If you fix a bug while restructuring code, you have done two things at once and only one of them is refactoring. Martin Fowler’s definition from his 2018 second edition of Refactoring: Improving the Design of Existing Code is precise on this point: a refactoring is a change made to the internal structure of code that does not modify its observable behaviour. Preserve bugs. Preserve features. Change only the structure.
Refactoring is not a sprint task you schedule once a quarter. By the time you need a dedicated sprint to clean something up, you have already let it get out of hand. The practical model is opportunistic refactoring: you clean up the code you are already working on, not code you found while browsing the repository at 2am.
The safety net comes first
Before you touch anything, you need tests that tell you whether the behaviour changed.
This is not optional. Without tests, refactoring is guesswork. You move a method, rename a parameter or split a function and you have no way of knowing whether you broke something other than running the application and hoping you clicked every path manually. That is not a process. That is a prayer.
PHPUnit is the standard testing framework for PHP. Laravel ships with it pre-configured. Before you refactor a method, write a test that captures what it currently does. Not what it should do. What it actually does right now, including any quirks you intend to keep in place. Run the test. It should pass. Now refactor. Run the test again. If it still passes, you have not changed the observable behaviour.
If you cannot write a test because the code is too tangled to test, that is the smell telling you where to start. The goal of the first refactoring pass is often just to make the code testable. Extracting a method that you can unit test directly is a perfectly valid outcome of an hour’s work.
The techniques
These five are the ones you will reach for most often. Each maps back to a smell from the previous article.
Extract Method
The smell it fixes: Long method.
The fix for a function that does too much is to pull pieces of it into smaller, separately named functions. The goal is that reading the outer function should read like a summary of the steps involved and each step should be readable on its own.
// Before: one method doing three distinct things
class OrderProcessor
{
public function process(array $items): float
{
$subtotal = 0;
foreach ($items as $item) {
$subtotal += $item['price'] * $item['quantity'];
}
$tax = $subtotal * 0.08;
$total = $subtotal + $tax;
if ($total > 500) {
$total = $total * 0.95;
}
return $total;
}
} // After: each step has a name
class OrderProcessor
{
public function process(array $items): float
{
$subtotal = $this->calculateSubtotal($items);
$total = $this->applyTax($subtotal);
$total = $this->applyBulkDiscount($total);
return $total;
}
private function calculateSubtotal(array $items): float
{
$subtotal = 0;
foreach ($items as $item) {
$subtotal += $item['price'] * $item['quantity'];
}
return $subtotal;
}
private function applyTax(float $amount): float
{
return $amount * 1.08;
}
private function applyBulkDiscount(float $total): float
{
if ($total > 500) {
return $total * 0.95;
}
return $total;
}
}
The outer process method now reads like a sentence. You do not need to understand the tax calculation to understand the structure. You can read each private method independently. Testing each step becomes straightforward.
Replace Nested Conditionals with Guard Clauses
The smell it fixes: Deeply nested conditionals that force you to hold multiple conditions in your head to understand what the function actually does.
Guard clauses exit early when a condition is not met. Instead of nesting your happy path inside multiple if statements, you return or throw as soon as you know you cannot proceed.
// Before: the actual logic is buried at the bottom
function processPayment(Order $order): string
{
if ($order->isPaid()) {
if ($order->hasItems()) {
if ($order->shippingAddress !== null) {
return 'ready_to_ship';
} else {
return 'missing_address';
}
} else {
return 'empty_order';
}
} else {
return 'unpaid';
}
} // After: guard clauses, happy path at the bottom
function processPayment(Order $order): string
{
if (!$order->isPaid()) {
return 'unpaid';
}
if (!$order->hasItems()) {
return 'empty_order';
}
if ($order->shippingAddress === null) {
return 'missing_address';
}
return 'ready_to_ship';
}
The before version requires you to trace the nesting to understand what happens in each case. The after version is scannable. Each failure case is handled immediately and the success case is obvious because it sits at the bottom without any indentation.
Introduce Parameter Object
The smell it fixes: Long parameter list.
When the same group of parameters travels together across multiple function signatures, they are trying to become an object. Create one.
// Before: seven parameters, order matters, easy to get wrong
function createUser(
string $name,
string $email,
string $phone,
string $street,
string $city,
string $postcode,
string $state
): User {
// ...
} // After: two clean parameters, address is its own concept
class CreateUserData
{
public function __construct(
public readonly string $name,
public readonly string $email,
public readonly string $phone,
) {}
}
class Address
{
public function __construct(
public readonly string $street,
public readonly string $city,
public readonly string $postcode,
public readonly string $state,
) {}
}
function createUser(CreateUserData $data, Address $address): User
{
// ...
}
The PHP 8.1 constructor promotion with readonly properties keeps the DTO concise. You can now type-hint Address wherever you need address data, validate it in one place and extend it without hunting down every function that previously accepted four separate string parameters.
Move Method
The smell it fixes: Feature envy, where a method is more interested in the data of another class than its own.
When a method spends most of its time calling getters on another object, the logic probably belongs on that object.
// Before: InvoicePrinter knows too much about Order internals
class InvoicePrinter
{
public function printTotal(Order $order): string
{
$subtotal = 0;
foreach ($order->items as $item) {
$subtotal += $item->price * $item->quantity;
}
$total = $subtotal + ($subtotal * $order->taxRate);
return 'Total: RM ' . number_format($total, 2);
}
} // After: Order owns its own calculation
class Order
{
public function calculateTotal(): float
{
$subtotal = 0;
foreach ($this->items as $item) {
$subtotal += $item->price * $item->quantity;
}
return $subtotal * (1 + $this->taxRate);
}
}
class InvoicePrinter
{
public function printTotal(Order $order): string
{
return 'Total: RM ' . number_format($order->calculateTotal(), 2);
}
}
Now Order knows how to calculate its own total. If the tax rate logic ever changes, you change it in one place. The InvoicePrinter does not care how the number is computed. It just asks for it.
Replace Magic Number with Named Constant or Enum
The smell it fixes: Primitive obsession, specifically raw values scattered through the codebase with no indication of what they mean.
A magic number is any literal value in code whose meaning is not immediately obvious. * 0.08 could be tax. * 0.95 could be a discount. status === 2 could mean anything. Name them.
// Before: what does 2 mean?
if ($order->status === 2) {
// process
} // After: PHP 8.1 backed enum, clear meaning everywhere
enum OrderStatus: int
{
case Pending = 1;
case Confirmed = 2;
case Shipped = 3;
case Cancelled = 4;
}
// If $order->status is stored as an int, compare against the backing value
if ($order->status === OrderStatus::Confirmed->value) {
// process
}
// Better: cast the int to the enum on the way out of the database
$status = OrderStatus::from($order->status);
if ($status === OrderStatus::Confirmed) {
// process
}
If $order->status is stored as a raw integer in the database, use ->value to compare against the backing value, or cast it to the enum with OrderStatus::from() on the way out of the database. The second approach is cleaner because once the status is typed as OrderStatus everywhere in your application code, the comparison is direct and the intent is unambiguous.
For numeric constants that are not a finite set of states, a named constant is sufficient:
// Before
$tax = $subtotal * 0.08; // After
const SST_RATE = 0.08;
$tax = $subtotal * SST_RATE;
A reader who sees SST_RATE knows immediately what that number represents. A reader who sees 0.08 has to guess. Do not make readers guess.
How to refactor without derailing the sprint
The most common reason refactoring does not happen is that developers treat it as a separate activity that requires approval. It rarely gets that approval. The backlog already has thirty items with business value attached. Cleaning up a service class does not ship a feature.
The practical model is the Boy Scout Rule: leave the code slightly better than you found it. Not dramatically better. Slightly better.
You are fixing a bug in PaymentService. You notice the method you are in is 90 lines long. You are not rewriting PaymentService. You are extracting the three lines that validate the payment reference into a private method called validatePaymentReference before you add your fix. That takes four minutes. Your test still passes. The next developer who opens that method finds it marginally easier to read.
This compounds over time. A codebase worked on by a team that consistently applies the Boy Scout Rule looks fundamentally different after a year compared to a codebase where nobody touches anything except what they are paid to change.
The opposite approach, scheduled refactoring sprints, tends to produce large diffs, heated code reviews and the occasional accidental regression because the changes are too broad to review carefully.
The pull request problem
Refactoring commits create noise in pull requests. A PR that mixes a feature, a bug fix and structural cleanup is hard to review. The reviewer has to mentally separate the behaviour change from the structural change to understand what is being added versus what is being reorganised.
The cleaner approach is to commit refactoring changes separately from behaviour changes. Even within the same branch, two commits tell a clearer story: “Extract validatePaymentReference” as one commit and “Fix payment reference validation for FPX transactions” as the next. The reviewer can look at the first commit and quickly confirm that no logic changed. They can then focus all their attention on the second commit, which is where the actual risk lives.
If your team uses squash merges, this separation disappears in the final history, but it still makes the review conversation easier while the PR is open.
One more practical point: do not title refactoring PRs “cleanup” or “misc fixes”. Those titles signal that the author did not think the work was worth explaining. Write what you actually did: “Extract order total calculation into Order model” or “Replace nested conditionals with guard clauses in PaymentController”. A clear title is reviewable. A vague one invites skepticism about what changed and why.
When not to refactor
Not everything that smells needs to be fixed right now.
Code that is stable, rarely touched and well-tested may be better left alone. The risk of introducing a regression through refactoring is real. A test suite that covers the behaviour gives you confidence. No tests at all means any change is a gamble, including structural ones.
Code that is about to be deleted is not worth refactoring. If a feature is getting removed next quarter, the cleanup cost will be zero. Do not spend two hours improving code with an expiry date.
The question is always the same: will the cost of fixing this smell be less than the cost of living with it? If the code is on a hot path, touched every sprint and actively slowing the team down, the answer is usually yes. If the code runs a report that nobody looks at, the answer is usually no.
The connection between smell and refactoring
Every code smell has a corresponding refactoring or set of refactorings. Fowler’s book is essentially a catalogue of smells mapped to the techniques that address them. Long Method points you toward Extract Function. Long Parameter List points you toward Introduce Parameter Object. Feature Envy points you toward Move Method.
The code smell article gave you the vocabulary to name what you see. This article gives you the first set of tools to do something about it. The two are not separate topics. They are the same practice, split into diagnosis and treatment.
The diagnosis is fast. You read the code and something feels wrong. The treatment is slower and requires care. But it is the treatment that actually changes the quality of what you are building.
