NA CO ZWRÓCIĆ UWAGĘ SPRAWDZAJĄC KOD PRZED URUCHOMIENIEM - DEVPARK
Development / Laravel

Code quality

No matter whether you create application for your own or you work in a team, no matter you work on a small project or a big one – you should always care about your quality. Obviously your code won’t be always ideal. You improve your skills every day (well, you should), programming language you use is changing, better coding standards are in the way (in PHP for example PSR-12) or new coding techniques appear that let you creating better, cleaner code. No matter of those, you should always try to make your code the best you are available.

Testing

Forget what I wrote before. You don’t need to care about your quality at all if the code you wrote won’t do what it should. Your client won’t care that your code is brilliant if the application has multiple bugs.

You should never trust yourself too much. Are you so great developer that you never make mistakes? Well, developers make mistakes, people do make mistakes so don’t expect the others make mistakes and you are the one who doesn’t.

One of the most important things in your code is – it should work. You should test your code if it does what it should, you should test the most complicated cases to make sure they won’t break the application.

The best way are of course automated tests. In PHP you can use PHPUnit or Codeception for example to write automated tests. They give you the easy way to verify if your new code won’t break anything. And if you add new code, you should write new tests yourself and run all existing tests to make sure everything is working as it should. This is unfortunately the way how we would like it to be. In fact tests won’t help you to find bugs if there are few tests or if they are written just to prove your code is working great.

Automated tests are the best option but sometimes you need to work in project without them. Also when you have tests you should not trust them so much when you write new code. So it means you should always test your code.
You can write a small function to launch your new code to make sure it will work (you will probably remove it right after you test your code), you can use Postman to test API action you’ve just created, you should test the change you made in website on your local machine.
If you have testers that’s great but it doesn’t mean you should deliver code with bugs just to let tester to find bugs you created. Also in case you don’t have testers, you should also test the code you added on develop server (assuming you have one) just to make sure your code won’t work only on your local machine but also on the others.

Code repository

I assume you use version control system (VCS) like GIT for example. If you don’t you should definitely start also in case you are the only developer or you write only for yourself.
To make it easier (also for you) to verify your code, you should always work on assigned task on dedicated branch for this task. So for example when you need to add new API action, you create new branch, you add commits into this branch, and when you finish working on this API action and have another ticket you should then switch to new branch and start working on this new branch.
This will let you (and the others) to look at your code and focus only on this one API action you added. Also in case you need to add any changes in your code, it will be much easier to add them into this dedicated branch.
General code quality

Let’s assume you created the code and it’s working. Does it mean it could be deployeopment server or production server? Well, in most cases it’s not ready yet. The first thing is you should review your code to make sure according to your knowledge something could be written in better way.

Shorter means better?

In general when you look at your code, very often you can make your code shorter. If you make your code shorter does it mean it will be better? In many cases yes but you should be reasonable with this.
Shortening 3 lines into 1 won’t be the good choice in case the new code won’t be obvious for you or other developers. On the other hand it will be the good choice in case you do something very basic that might be already built in application you develop or in general shorter notation will be cleaner.

For example in Laravel application we could write it like this:

There might be nothing wrong with this, but in most cases the better option would be:

Instead of 4 lines we have now one line, we use built-in method and it’s easier to read it. So after the change code is definitely the better one.

Code duplication

When you develop new functionality often you don’t choose the best way right away. You play with code, you add new function, sometimes you copy a piece of code from other part of application. There is nothing wrong with that but finally you should clean up this mess.

Code duplication is the thing that should be avoided as much as possible. Having the same 3 lines of code in 10 different parts of application is not the best way. What if you need to change something in the behavior? Are you going to put those changes into those 10 different parts of application? Or maybe you will change them in 8 places and you will forget about additional 2?

Code duplication is very hard to maintain. The code duplication is not when you just duplicate the exact same code but also in case you could create additional common function with parameters and you don’t. For example:

If you look at above code, it’s not exactly the same. In one method we use user_id in 2 places but in the other one we use project_id in 2 places. Is this code duplicated or not? Well, yes it is. There’s no need to do it like this.

You can create method like this:

and now you can reuse this code like this:

Do you see the difference? Code duplication disappeared.

In real life I saw for example 20 lines of code duplicated 10 times and in each of duplicated block the only difference was the different number in MySQL query To refactor this, it was necessary to compare each 20 lines just to make sure the only difference was the single number in it.

Code organization

The next thing is code organization. Obviously you can put all the code into single file. But is it really the best way? You should divide your code into logic blocks – use functions, use classes and don’t make them too long. I’m not a big fan of creating classes with single method only but sometimes it might be also the good way in case the code is doing something very specific and it’s likely this functionality will be more complicated in future.
When we are talking about code organization it’s not always about just refactoring single function to make it cleaner and separated into a few functions. You can make also micro-scale code organization that will make your code easier to read and easier to change.

Let’s assume for example you have your Article model and you have published_at field. This field contains date when article has been published or null in case it has not been published yet. To decide whether you should notify someone that the article has been published you could write something like this:

There is again nothing wrong with this, but I believe we could do it better. The condition is very simple (and it could be simplified in this case) but what in case you should repeat it in 10 other places? What in case you would like to change this condition in future and to make it like this for example:

You would again make the change in 10 places of your application. So it’s much better to organize your condition in different way at the beginning. You can create for example such method:

It’s easier to read now and in case this condition is or will be more complex we could put the logic into this single function instead of need to make changes into many places of application.

Choosing the right names

Choosing right names is one of the most important things. You should choose right names for variables, functions, classes, interfaces and traits. This is really important, if you use invalid names you can make your code very hard to read and maintain and you can cause also bugs in your application. Let’s look at some examples:

In above code method name is valid, but variable name $users is invalid. If someone works with this code they will think $users variable contains collection/array of users and not single user.

And looking at such piece of code

we can see, name of calculateSum function is completely invalid. It not only calculates sum but also change the items of given array. It could lead to serious bug if any developer would like to use this function without looking what this method really does (it should only calculate sum looking at the function name, right?)

Comments

Another important thing is comments. I’m not talking about docblocks (I personally use them in my code) but generally about commenting your code. If you choose valid names for variables and functions I can agree those commands are not that necessary, but this is not always the case.

For example look at this code snippet:

As you see there is comment here added. For me this is obvious that validation would be fired here. However I assume for many other developers if there was no such comment they wouldn’t know that validation would be fired automatically in this line.

So when you think about comments, think of them as hints for other developers or for you if you will get back to this project after 10 years.

Refactoring

Refactoring code you write before deploying on server is really obvious. I’ve already covered code duplication and code organization things so you know you should verify your code like this and make refactoring if needed. But there’s something more.
When you develop new functionality it might happen that you copy a piece of other code and you make small changes. Each time you do something like this, you should decide whether you cannot reuse this code. Maybe you could create a function and reuse it in your code that already exists? Or in case you would like to use a piece of code that is not in separate function, maybe you should create separate function for this piece of code?
However when you make refactoring the most important thing is – don’t break anything. If you are not sure what a piece of code does and you are not fixing the bug in it, sometimes it’s better not to touch this.

Code style quality

The last thing you could improve in your code is code style quality. It’s really good to have some code style rules at least for yourself in case your company don’t force you to particular code style. You should decide for example whether you use camelCase or snake_case convention for variables and you should follow this convention in the whole project. You should also follow PSR-2 if you use PHP but it doesn’t cover everything.
It’s very useful to have some tools that will help you to have better code style quality and it’s really important to have them when multiple developers work in same project. You could use for example PHP CS fixer or PHP CodeSniffer for PHP projects. They will make sure your code will be formatted in the same way also in case some developers don’t know code style rules you want to follow in this project.

Pull requests

When you finish work in your VCS branch you should never merge your code directly into develop/staging or master branch. I know many developers do this but no matter if you work in the project alone or in a team, you should create Pull request.
When you create new pull request, you are telling that you would like to merge the changes you made in your branch into other branch (for example develop). And once you created Pull request, you are telling everyone that your code is as good as possible, it’s working and in your opinion it’s ready to be merged.
If you are working in project alone after creating Pull request you could merge this Pull request right away (in case you follow other techniques from this article) and in other cases probably Senior developer or Project manager will review your code. In case everything is fine, your code will be merged and in case there are some problems, they can add comments and you will make changes in your code and create new Pull request once everything was fixed.

You can create Pull requests without any problems in popular GIT systems like Github, Bitbucket or Gitlab so in case you haven’t used this feature yet, I recommend you start right away.

Techniques to review your code yourself

TODO is your friend

When working on new functionality you often miss some things. Sometimes you don’t add validation layer because you focus first on most successful cases, sometimes you don’t add additional condition although you know it will be necessary before deploy.
I suppose each of us don’t do everything right away, but the important thing is not only to remember about those necessary things but add todo comment in your code for example:

// @todo add validation

or

// @todo add transaction

or

// @todo what if there is no user set?

When you write code like this it will help you a lot. First of all I’m pretty sure if you need to remember so many things without adding any comments, finally you will forget some of them. What if such code will be deployed on production? Do you really trust your memory so much?
Also be aware that you should always expect something unexpected could happen. What in case your boss tells you – stop everything you are working on right now and start working on another project. Are you sure after a week you will be able to continue and you will still remember all things you wanted to add?
And what in case it won’t take a week but a month? What in case other developer will need to continue your work in this project? Maybe he will see that he needs to add validation or transaction in above case, but maybe he won’t see extra condition that should be covered too (when there is no user set).

So to sum up, you should always add todo comments every time you miss something and you think you will add this later.

Small commits

When working on new functionality you should probably never wait until all the code is done to create commit. I personally create multiple commits for single functionality, sometimes I break one commit into multiple just to make the change more visible.
For example when I’m working on new functionality and I’ve found a bug in my previous code, I create small commit with this bug fix. Obviously I add descriptive message so when I (or someone else) look in GIT history I will see that bug was fixed.
It’s really important to add descriptive messages to your commit. When you only add message “Bug fix” – what does it mean? Probably nothing. Also in case you work with ticket and you will add “TICKET-123 – bug fix”, when you look at this commit message you have no idea what really happened. It’s much better to add message like this “TICKET-123 – fix bug with invalid query string”.

But why it’s good to have smaller commits? It’s much easier to look yourself at smaller change and decide if everything is fine with this commit or something should be fixed (see next section).

Review before commiting 

When creating commits you should review the code you want to commit. You should review all things from General code quality and you should review your code style quality (or run tools that will do this for you). In general you should make sure the code you commit is as good as possible.
That’s why it’s really useful to have small commits. When you have small commit, you can review the changes very quick, you can add necessary doc blocks or comments to make sure everything is they way as it should be. And in case you have 10 files with multiple changes it’s much harder to do this.

Merging your code with recent changes

When you work in project where multiple developers are working, when you created your branch you are not synchronized with recent repository changes in the branch. Let’s assume you created new branch from develop branch and later you want to merge your changes again into develop (via Pull request) and you are working in a project with 4 other developers. You are working on the functionality for 4 days.
But during those 4 days, what is in develop branch could be completely different. Other developers could push many many changes in such big period of time or remove class you rely on. That’s why you should make sure during your work to merge changes from develop branches into your branch.

I recommend to synchronize changes from target branch to your branch at least once a day. Obviously when you start your new branch from target branch you should make sure you are creating your branch from up-to-date target branch and not the one that was for example 2 weeks ago.

Creating Pull request

When you are going to create Pull request, the first thing is to make sure you have all changes from target branch in the branch you would like to merge into target branch. As I already said you should do it also when working on your branch, but before creating Pull request this is a must.
Next thing are tests. If you have any automated tests you should run them before creating Pull request to make sure they all pass. Of course you should run some tests when working on your branch, but again before creating Pull request you should run again all the tests on your local machine.

Now, when you start creating Pull request, before you finally create it, you should review all the changes yourself. In all GIT systems like Github, Bitbucket and Gitlab when you start creating pull request you can see all changed filed and diffs so this is perfect place for you to review your own code.
Although you reviewed each commit separately it doesn’t mean everything is fine. First of all, you could have missed something in your code so now you have chance to fix this. It’s also possible that when you finally look at the all changes you made, you could decide it’s better to do something in the other way. You can find for example some code duplication because you implemented some big condition in one commit and in other one you repeated it and now you could decide to refactor this condition into some function for example.
In case you decide you should make some changes in your code instead of creating pull request, you could make some changes in your branch and then again start creating your Pull request. Sometimes such process could be repeated multiple times because next time you will find other issues when you start creating Pull request you haven’t found before.
When you finally think the code you created doesn’t have any issues, you can create Pull request and either merge it to target branch (in case you are the only developer or you don’t have reviewers in this project) or to wait until Pull request is approved (or declined) by reviewer.

Benefits

There are many benefits of attitude described above. First of all, you as developer try to make your code as good as possible. You review each your commit, you review the whole Pull request, you improve your skills each day.
When you are working both as developer and reviewer of your code you will see, how the final reviewer work looks like. Probably at the beginning you can find multiple issues in your code yourself but after a while maybe you will get into perfection.
When you as a developer review your own code and try to make it perfect you save company’s money. You should assume that reviewer is someone with bigger experience than you are (and also with higher salary) so it’s better when you fix some obvious issues before reviewer will review your Pull request.
Finally you make that your code will sooner be merged into target branch what means again huge savings to your company.

author: Marcin Nabiałek, leading programmer at Devpark Laravel Team