loading...

5 bad habits to lose in PHP

klnjmm profile image Jimmy Klein Updated on ・2 min read

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. I'm also writing a dev letter that you might like.

Posted on by:

klnjmm profile

Jimmy Klein

@klnjmm

Hi, it's Jimmy πŸ‘‹, PHP 🐘 and React βš›οΈοΈ developper, guitarist 🀘 and minimalist apprentice.

Discussion

pic
Editor guide
 

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.

echo "Hello {$name}" ;
 

String interpolation works to in this case. πŸ‘
I am used to (s)printf when
I 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

printf('<a href="%s"></a>',$link);

vs

echo "<a href=\"{$link}\"></a>";

Of course one could write it this way

echo "<a href='{$link}'></a>";
  • but for some reasons I don't like single quotes for html attributes :)

At the end and in this case, whatever works best/is the easiest to write and read. sprintf with 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