Polishing your code
While preparing content for my upcoming PHP Package Development video course I discovered that over the last couple of years I spend more and more time polishing my code before shipping it.
And I believe that this is something that is just as important as the actual coding itself.
Let me talk you through some of the adjustments that I try to make, while working on my codebase and how this can hopefully affect your way of writing good and readable code.
Clean Code. And what it means to me. #
So, let's start off with a quote from Michael Feathers, out of "Clean Code".
“Clean code always looks like it was written by someone who cares."
This is one of the best quotes around the topic for me, as I think it completely nails the concerns of "clean code".
When you look at a piece of code, you can tell if someone cared about writing it or if someone just rushed it and didn't really give it a second thought.
Let's see some examples and how you can use them to write better code.
Meaningful Names, Mental Mapping, and more #
There are only two hard things in Computer Science: cache invalidation and naming things.
-- Phil Karlton
Now as developers we all know that coming up with good names for our function names, variable names, class names and even domain logics is a hard task. How many times have you written code that involved a $data
variable? What does that even mean? It could be anything.
So let's say you have a class that has a method to get all active blog posts out of a database.
Here's an example of how not to write this:
class BlogPostReader
{
public function getData(): array
{
$result = [];
$data = $this->fetchDatabase();
for ($i = 0; $i < count($i); $i++) {
if ($data[$i]->active === true) {
$result[] = $d;
}
}
return $result;
}
}
Alright..now there's a whole lot going on these few lines of code. Before we're going to see what's wrong with this, let's see the same code written in a better way:
class BlogPostReader
{
public function getActivePosts(): array
{
$posts = $this->getAllPosts();
return array_filter($posts, function ($post) {
return $post->isActive();
});
}
}
What have we done here?
We now have a meaningful method name #
Instead of just calling a getData
method, that could return anything, we've renamed the method to getActivePosts
- because that's what the method is doing. Do not be afraid of using longer method or variable names. You're probably using an IDE of some sort to help you writing it anyway and if it helps you and other developers to better understand what's going on, that is always more helpful than saving a few keystrokes.
No mental mapping #
The loop that we had in the first example added a mental mapping to everyone that reads your code. You have to remember which variable holds which information and remap it in your head. Explicit is better than implicit.
Move the comparison into a function #
First of all, the first example was using simple comparison with only two equal signs instead of identical comparison, which would use three equal signs. Especially when dealing with PHP (or Javascript) this will save you a lot of trouble since you don't get false positives because of string/integer conversions for example.
Next I've moved the comparison into the $post
object to make this re-usable accross the application.
Now for the sake of this example, I added the isActive
check - in a real application it might make more sense to only fetch the active posts in the first place, instead of looping over them.
Avoiding unnecessary complexity #
This is a modified example of our Laravel Websockets package at the time we were writing it:
public function handle()
{
WebSocketsRouter::echo();
$loop = LoopFactory::create();
$routes = WebSocketRouter::getRoutes();
app()->singleton(HttpLogger::class, function () {
return (new HttpLogger($this->output));
});
app()->bind(ConnectionLogger::class, function () {
return (new ConnectionLogger($this->output))
});
$websocketServer = new WebSocketServer($routes);
$websocketServer->setHost($this->option('host'));
$websocketServer->setPort($this->option('port'));
$websocketServer->setLoop($loop);
$websocketServer->run();
}
When you look at the code above, can you clearly identify what this method is doing? You probably can't. At least not easily. So this method adds a lot of complexity to whoever is reading it because you can't immediately tell what is going on.
To avoid this, I like to split my code up into a lot of smaller methods. Don't be afraid of using multiple methods in your code. You should aim for small and readable methods. This makes it a lot easier to understand the code. This way you can also avoid having to add unnecessary comments - as the code should be as self-explanatory as possible.
So in our example, we moved the different calls into separate functions, so that the handle method looks something like this in the end:
public function handle()
{
$this
->configureHttpLogger()
->configureConnectionLogger()
->registerEchoRoutes()
->startWebSocketServer();
}
This is so much better. When you look at the code you can immediately see what is going on.
First we configure the HTTP logger, then the connection logger, then we register Laravel Echo routes and finally we start the WebSocket server.
And if you look at the individual methods, those are now really short so that it becomes way more maintainable:
protected function configureHttpLogger()
{
app()->singleton(HttpLogger::class, function () {
return (new HttpLogger($this->output));
});
return $this;
}
protected function configureConnectionLogger()
{
app()->bind(ConnectionLogger::class, function () {
return (new ConnectionLogger($this->output));
});
return $this;
}
protected function registerEchoRoutes()
{
WebSocketsRouter::echo();
return $this;
}
Vertical density and vertical distance #
One other thing that I want to point out is called "vertical distance" and "vertical density". It's also discussed in depth in the "Clean Code: A Handbook of Agile Software Craftsmanship" book.
At first I thought it's just nitpicking on the codebase, but it really started to grow on me and makes me look at code in a different way.
So the main concept is:
Things which are close together go together.
And of course the reverse is true: if things are physically separated and you look at them, you assume that they are not as strongly related a a proximal connection.
When you look at a piece of code from top to bottom you want it to be readable as a newsletter. You have the important things at the top and the more you scroll down, the more detail information you can read. That's why we place all class properties at the top. PHP does not matter if you place them at the top or in between.
So here's an example of how not to structure your code. I couldn't place the class property in between - that would be too much for me hah.
class Employee
{
protected $id;
protected function isFullTimeEmployee() {}
protected function getSalary() {}
protected function calculateBonus() {}
public function getId() {}
protected function didCompleteBonusGoal() {}
public function calculatePaycheck() {
$salary = $this->getSalary();
if ($this->isEligibleForBonus()) {
$salary += $this->calculateBonus();
}
return $salary;
}
protected function isEligibleForBonus() {
return $this->isFullTimeEmployee() && $this->didCompleteBonusGoal();
}
}
So if you look at the code above, everything is just all over the place. Methods that belong together/get called should also be close to each other in your code.
Also, there is no spacing in the calculate salary method. Give your code some breathing room!
Alright, let's take a look at a better structured code:
class Employee
{
protected $id;
public function getId() {}
public function calculatePaycheck() {
$salary = $this->getSalary();
if ($this->isEligibleForBonus()) {
$salary += $this->calculateBonus();
}
return $salary;
}
protected function getSalary() {}
protected function isEligibleForBonus() {
return $this->isFullTimeEmployee() && $this->didCompleteBonusGoal();
}
protected function isFullTimeEmployee() {}
protected function didCompleteBonusGoal() {}
protected function calculateBonus() {}
}
Now when you take a first look at the class you can clearly see that you can call a getId
method and you have a calculatePaycheck
method - that's nice.
Now if you want to dig deeper and want to try and figure out how the paycheck get's calculated it's easy - the methods that get called are vertically close to our calculatePaycheck
method - because they belong together. So all you have to do is keep reading your code to understand it better.
You can also read more how and why this works for our brains here.
Writing clean code #
As I've said there are a lot of additional best-practices and things you should try to focus on when you want to write clean code - no matter the programming language.
One great way to get started is by reading the Clean Code book, as mentioned above. In addition to that you can check out the Clean Code PHP repository on GitHub that covers additional aspects along with small PHP code snippets.
When I'm writing my code, I don't always stick to every clean code rule immediately when writing the line of code. But I try to take the time to go and read-through every line again before "shipping" it in one way or the other.
This could be done before doing a git commit
, or before merging a feature branch into your application, or before publishing and tagging the first version of your open source package.
As Michael Feathers said: you want your code to be written so that you know that you cared about it. That's the most important fact.
Learning more about PHP package development #
As I mentioned, I have started working on a new video course called PHP Package Development that is set to be released early this year and show you how to create your own reusable PHP packages for yourself, your company or for the whole world on GitHub.
If you are interested in learning more about PHP and Laravel package design, be sure to sign up and get notified when the course launches, as well as receive a launch discount code.