Recently, I was hired to refactor a huge codebase that was messed up for years. The project was a Magento 2 project with more than 30 custom modules. The modules basically did everything you shouldn't do in Magento. I didn't find any tests or documentation. No static code analysis, nothing. The fact, that there aren't any tests is the reason for the messed up code base.
While I was studying the code, I stumbled over the same ugly patterns over and over. This article is a small summary of the 5 worst patterns.
Chaining... Look at the following:
Chained function call are hard to debug, harder to read and difficult to understand. While debugging, you have to evaluate each of the function calls or step into them instead of just jumping over them...
A better approach would be:
It might be a preference of some, but for me yoda style is one of the most unreadable style ever. Maybe my brain is not fast enough to evaluate the condition, maybe yoda style is the pure evil.
Do me a favor and stop doing it <3.
This feels so much more natural:
Fail fast, fail loud. That's a common statement when writing public functions. I often see functions doing a lot of stuff before they fail because a given parameter has a value that was not expected. Fail fast, fail loud. Validate incoming params first, fail if they're not matching the requirements of the functions, otherwise continue;
Invert the conditions and add early exits.
When an if block is guaranteed to exit control flow when entered, it is unnecessary to add an else statement. Exits can be returns, throws, continues, breaks.
I would say, that else-statements aren't necessary in most cases. Isn't the following code much better to read?
Old but gold. People still give a sh*t about how they name things. Naming is hard, I know. Nevertheless, here are some rules of thumb for many use cases:
Suffix Class Names with their purpose.
Try to find meaningful variable names and add the following names to your variable-name blacklist:
- $data --> $productData
- $params --> $requestParams
- $model --> $product
- $message -> $userFlashMessage
- $id --> $cartId
- $item --> $cartItem
- $connection --> $redisConnection
Explicit function names. We all know, that our functions shall only do one thing. This in mind, there can be functions similar to each other but having different interfaces. Let's imagine a CustomerMapper Class, mapping customer data. Methods can be named like:
For sure, we all work with return types nowadays but it's still a good idea to declare function names that return a boolean like so (Thank for the hint Pujan Srivastava)
- hasStock() : bool
- isGuest() : bool
- canBeAddedToCart() : bool
Disclaimer: Snippets are meaningless examples.
Have an additional tip? Looking forward to hear it