I do a lot of code review and I often see the same mistakes. Here's how to correct them.
Test that an array is not empty before loop
$items = [];
// ...
if (count($items) > 0) {
foreach ($items as $item) {
// process on $item ...
}
}
foreach loop or array function (array_*) can handle empty array.
- No need to test it before
- One less indentation level
$items = [];
// ...
foreach ($items as $item) {
// process on $item ...
}
If you want to learn how to code without for/foreach/while loop, I recommend my article on collections (in French).
Encapsulate all the content of a method in an if statement
function foo(User $user) {
if (!$user->isDisabled()) {
// ...
// long process
// ...
}
}
This one is not specific to PHP but I see it very often. I have already talk in my article about calisthenics objects and in my article about my minimalist code of reduce indentation level using early return.
All the "useful" body of the function is now at first indentation level
function foo(User $user) {
if ($user->isDisabled()) {
return;
}
// ...
// long process
// ...
}
Call multiple times isset method
$a = null;
$b = null;
$c = null;
// ...
if (!isset($a) || !isset($b) || !isset($c)) {
throw new Exception("undefined variable");
}
// or
if (isset($a) && isset($b) && isset($c) {
// process with $a, $b et $c
}
// or
$items = [];
//...
if (isset($items['user']) && isset($items['user']['id']) {
// process with $items['user']['id']
}
We often need to check that a variable is defined (and not null). In PHP, we can do this by using isset function. And, magic, it can take multiple parameters !
$a = null;
$b = null;
$c = null;
// ...
if (!isset($a, $b, $c)) {
throw new Exception("undefined variable");
}
// or
if (isset($a, $b, $c)) {
// process with $a, $b et $c
}
// or
$items = [];
//...
if (isset($items['user'], $items['user']['id'])) {
// process with $items['user']['id']
}
Combine the echo method with sprintf
$name = "John Doe";
echo sprintf('Bonjour %s', $name);
This bit of code may be smiling but I happened to write it a while ago. And I still see it quite a bit! Instead of combining echo andsprintf, we can simply use the printf method.
$name = "John Doe";
printf('Bonjour %s', $name);
Check the presence of a key in an array by combining two methods
$items = [
'one_key' => 'John',
'search_key' => 'Jane',
];
if (in_array('search_key', array_keys($items))) {
// process
}
Last error I see quite often is the joint use of in_array andarray_keys. All of this can be replaced using array_key_exists.
$items = [
'one_key' => 'John',
'search_key' => 'Jane',
];
if (array_key_exists('search_key', $items)) {
// process
}
We can also use isset which also check that the value is not null.
if (isset($items['search_key'])) {
// process
}
Thank you for reading, and let's stay in touch !
If you liked this article, please share. You can also find me on Twitter/X for more PHP tips.
Top comments (7)
The multiple isset is a good one, thanks
Note that the "early return" is called a "guard clause"
For the fourth one, I rather prefer string interpolation.
String interpolation works to in this case. 👍
I am used to
(s)printfwhenI want to translate string which contains variables with gettext and also to format float.
I like string interpolation too whenever I can get away with it.
String interpolation gets weird if your string contains quotes, though
vs
Of course one could write it this way
At the end and in this case, whatever works best/is the easiest to write and read.
sprintfwith many vars is a pain to read.I don't usually program in PHP anymore but just today I had to do something and I made the multiple isset mistake. Thanks! I learned something new