This series of articles is taken from a talk I gave at the Berlin PHP Meetup and was originally posted on my blog. If you didn't read the first one, here you go.
Dev.to has some issues with my Gists. It scrambles them! You should definitely read this article here instead.
Write explicit code
Last time we talked about the strategies you could set up to share knowledge within your current coding team. It's great, but it would be even better to help anyone to read your code and understand your implementation right away. Let's dive into some concrete code.
Let's say you are new on a project and you have to add a new feature in this part of PHP code below. I know, it's PHP. But don't leave. It's a basic one, and even if you hate that language, it should not be a problem to understand what is going on. These considerations should apply to almost any language and code. This is a dummy example I wrote, to reflect the kind of usual bad code you can find when you join a random project.
Try to understand what this code is about, in a few seconds.
Ok, I think you got it: the previous code is about calculating shipping fees. But that was not so obvious to get, and still, it is a very simple and short class. Imagine how it can feel for a newcomer in your project if there are hundreds of classes like this, and five times as long as this one.
Let's try to clarify that.
First, the naming is very bad. Service does not mean anything and the method name is not explicit enough. We rename the namespace, the class and the method (l. 3, 5, 14):
Better. Now the next problem is that a Calculator should calculate and return a value. Here, it calculates and sets the value to an object. This is misleading, and multiplies the responsibilities of the class. It also finds the Cart object from a repository: it is quite hard to defend the injection of a repository in this simple Calculator class. Let's directly pass a Cart object as parameter (allowing an explicit typehint) and just return the calculated value (l. 9, 29).
This if/elseif is confusing and takes too much of my energy to follow. Let's get rid of that (l. 21 and 29):
I feel better already. This calculation of the total of the cart could be the responsibility of the model. Then let's get rid of it too, and add an explicit method name so that we know what total we are talking about (because yes, when calculating prices, knowing if we include VAT or not can be interesting) (l.13):
I still have an issue here. What are all these magical numbers? 20? 40? 100? If you are new to the project, chances are that you will be a bit afraid to change one of them and will ask to your client or your colleagues what they actually mean. If they are supposed to change, we inject them as parameters with clear variable names. If they are not, then we can use constants (l. 11, 17, 22, 25):
This could look a bit overkill to use constants here, but it will help for sure any new comer to the project to understand right away what is going on. Even better: if you have a question about these numbers, you can directly ask your client about the threshold for free shipping fees instead of making them guess about this 100 number that could be anything in the app. Finally **it allows you to centralize these informations somewhere and reuse them in the app, avoiding you some headaches while refactoring.
This is way better. But I still have a last issue here. I guess these numbers are some money. But with what currency are we working here? Euros? Dollars? Yen? If you clarify this right away, that will prevent every developer from being lost when the app is growing. For this, we can use the Money library which implements Martin's Fowler Money pattern. We also add a return typehint, since we can do it since PHP 7.1 (l.5, 14, 18 and 26):
If it is maybe as verbose as the initial code, I think this code is much easier to read, to understand and to maintain. You understand right away what we are talking about and what it does, without the need of any comment. This will also (in my opinion) help a lot of new comers in your project to dive into your code and be able to refactor or maintain any class. It will avoid the team to waste time with infinite questions, since the code is self-documented. This is obviously a very simple example I took for the purpose of this article, but I think this way of refactoring and writing explicit code can apply to much fatter classes. Also this Calculator is a dummy example: we could (should?) do that directly in the model instead of instantiating a service for this simple task. Once again, it was just a straightforward example to show a few common problems we can find on a daily basis.
Next time, we will talk about one of the most important issues when lacking of knowledge sharing: the domain.
Picture credits: "Juventae Chasma" by European Space Agency.
Top comments (0)