A month of clean code
Tags: programming, practices, refactoring, php
Published: 2017-11-10 11:35:28

---
title: A month of clean code
published: true
description: A collection of my recent "clean code" tweets with before and after code samples.
tags: programming, practices, refactoring, php
cover_image: https://thepracticaldev.s3.amazonaws.com/i/v3qxt9u73pahbt6u4cd3.png
---
Each week over the past month, I have posted _before_ and _after_ code samples calling out ways to clean up code. The response has been amazing, many receiving hundred of likes. While I will continue to share these weekly clean code samples, I wanted to collect the previous ones into a single post.


## Breaking up long methods
In this [first tweet](https://twitter.com/gonedark/status/918576634755731463) a developer submitted a long Laravel controller action they wanted to clean up. At a high level the controller action handled a course enrollment request. At a low level the code had two paths for enrolling the user.

![Before and after code cleanup of a long method](https://thepracticaldev.s3.amazonaws.com/i/nlrp1nhwrubhu4b3mqd6.png)

The main question to ask when evaluating a long method is _who is responsible for this section of code_? The answer to this question will help determine if the code can be moved elsewhere. In this case, the creation of the user could be moved to the model. We could also streamline the two paths by creating a private, conditional method. This method performs the branched logic of creating the user when necessary.

The result is a much smaller method. Some may argue we've simply moved the code. Nevertheless, the intent of our original method is more concise and its component parts are easy to follow. Both improve communicates - which is [the measurement of clean code](https://dev.to/gonedark/writing-clean-code).

As pointed out on Twitter, the _after_ code did have some mistakes, such as missing parameters to `createUserIfUnauthenticated`. However, these are trivial mistakes which do not change the clean up.

{% twitter 918576634755731463 %}

---

## Leveraging objects
The [second tweet](https://twitter.com/gonedark/status/920322516333350912) was a Laravel controller action submitted by a developer. At a high level the controller action handled storing image uploads. At a low level the controller did everything itself - validation, image manipulation, storage.

![Before and after code cleanup introducing objects](https://thepracticaldev.s3.amazonaws.com/i/zsjw5rp1l1yt4rgprgi6.jpg)

To clean this code, the strategy is to leverage other objects to handle _all the things_. A [Laravel Form Request](https://laravel.com/docs/5.5/validation#form-request-validation) object could be used for the validation. We could adopt the Repository Pattern to introduce an object to coordinate the storage of an image.

Similar to before, the result is a smaller method. What's different in this case is leveraging existing objects available in the framework. We also applied a pattern that fit our code. Too often developers do this in reverse - fitting the code to a pattern. Letting a pattern emerge from the code is a far _cleaner_ approach.

{% twitter 920322516333350912 %}

---


## Cleaning conditional code
The [third tweet](https://twitter.com/gonedark/status/923557577593458689) focused on cleaning up the conditional code that forms most of our codebase. This has been the most popular cleanup so far. Likely because the fundamental code allows it to apply to many languages.

![Before and after code cleanup of conditional code](https://thepracticaldev.s3.amazonaws.com/i/v3qxt9u73pahbt6u4cd3.png)

Most of these cleanups fall under the third _Writing Clean Code_ practice - [avoid nested code](https://dev.to/gonedark/writing-clean-code). Anytime you want to clean up conditional code as yourself two questions:

1. Can I return the condition directly?
2. Does inverting the conditional logic improve communication?

{% twitter 923557577593458689 %}

---


## Spotting primitive obsession
The [fourth tweet](https://twitter.com/gonedark/status/928638252516544513) focuses on spotting [Primitive Obsession](https://refactoring.guru/smells/primitive-obsession). Our obsession with using primitive data types often results in duplication of related code. It's not the exact same code which makes it harder to spot. Over time this code leaks through the system and becomes harder and harder to cleanup.

![Before and after code cleanup of primitive obsession](https://thepracticaldev.s3.amazonaws.com/i/xl9ppip19z7an5gndnry.jpg)

The tips do not draw out some additional characteristics of these _value objects_. Almost as important as the encapsulation there provide is the fact that they are immutable. This limits state, making them relatively lightweight to add and even easier to maintain. For more detail and examples on `Range`, read [RangeObject by Martin Fowler](https://martinfowler.com/eaaDev/Range.html).

And yes, I would absolutely refactor `limit()` to utilize array filtering. This would be the next clean up:

```php
function limit($items, Range $range)
{
    return array_filter($items, function ($item) use ($range) {
        return $range->includes($item);
    });
}
```

{% twitter 928638252516544513 %}

---


_**Want more cleanup tips?** [Follow me on Twitter](https://twitter.com/gonedark) to get weekly tips or sign up for my upcoming [field guide](https://basecodefieldguide.com)._