Refactoring #1: Using ternary and null coalescing operators in PHP

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.

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.

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.

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.

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.

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 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.