Refactoring #1: Using ternary and null coalescing operators in PHP
Tags: laravel, refactoring, php, webdev
Published: 2021-07-17 16:13:46

Take a look at this little method below. Three issues come to mind the first time you see it. Unnecessary calls to `array_key_exists`, long chaining of `if` conditions, and a redundant nested code. Let's see how it can be improved, without changing its behavior.
{% gist https://gist.github.com/GeniJaho/c084a5adc2e00ecee2535bc163603ecb %}
## Using the null coalescing operator
The first thing we want to do is get rid of all those calls to `array_key_exists`. The reason for that is that it can be written much more simply. The null coalescing operator is meant for exactly the same purpose we have here, to be safe from exceptions of missing array keys.
{% gist https://gist.github.com/GeniJaho/9289a6ac1b212639a48682d90b121d9c %}
## Using the ternary shorthand operator
The code basically wants to retrieve a `$cityName` value from the `$addressArray`, which has many keys to represent the name of a city. It traverses the array keys one by one until it finds a non-empty value. It can be made shorter, however, by replacing the `if` conditions with the ternary operator `?:`. The ternary operator will return the left-hand value if it evaluates as `true`, otherwise, it will return the right-hand side.
{% gist https://gist.github.com/GeniJaho/c8fabec1beada9670bd23402ba324548 %}
## Removing one more unnecessary condition
At the end of these empty value checks, the code has one more condition. If the value is still empty, it sets `$cityName = 'error'`;. Then, only if this value is not `'error'`, it allows the "happy path", which is fetching or creating the city, launching an event, and returning that `$city`.

Now either the programmer wanted to make sure not to proceed with the happy path if they got an empty value, or they wanted to prevent getting a value of `'error'` from the `$addressArray` itself, which is unlikely. Taking into account how the rest of the code is using this method, it's safe to say that they simply wanted to avoid empty values. In that case, we just check for empty values.
{% gist https://gist.github.com/GeniJaho/5f87aedd5e8e1b4bd7c1f6610c74cbec %}
## One more nested code gotcha
The code above looks good, but it can look a little bit better. That `if` condition is visually pushing the main bulk of code on the right of the screen, although by a very small margin. But what if the main code was longer, and more complex?

This happens pretty commonly, checking for the right conditions to execute the happy path, and if those conditions aren't met then return an error value. To fix this, you should "exit early", which means you test for the error condition (or exit condition) first. If that condition is met, exit right away, and the rest of the code can run without being nested.
{% gist https://gist.github.com/GeniJaho/121ebbd1d9e31924aacef7dbc88567f7 %}

And that's it. These little changes mostly help for making the code more readable, and a little bit shorter.

> The code used for illustration is taken from the [OpenLitterMap](https://github.com/OpenLitterMap/openlittermap-web) project. They're doing a great job creating the world's most advanced open database on litter, brands & plastic pollution. The project is open-sourced and would love your contributions, both as users and developers.