Bunq-5-common-mistakes-php-coders
‹ Back to blog

5 Common Mistakes PHP Coders Make

At bunq we use PHP because it's easy to learn and flexible, but that can leave room for mistakes. Here's 5 things to keep your eye on when you build your bank in PHP.

The Background 🧠

Hi there! My name is Thijs and I’m a 23-year-old backend developer working alongside a team of around 20 very capable and incredible coders who are literally building the Bank of The Free.

At bunq, almost all of our code is written in PHP. Why you may ask? Well, mostly because it is easy to use, easy to learn, and very flexible. But it's precisely because of this flexibility that mistakes are easy to make.

Don’t worry, we all make mistakes, and all (PHP) developers have made them at some point in their career. So whether you’ve been using PHP for 5 years or 5 months, it’s always good to keep an eye on where you can improve.

Using 'isset' incorrectly 🤔

isset is a wonderful function which not only allows you to check whether a variable has been defined but also lets you check whether a key is set in a given array. Now consider the following code example:

$body = [
 'age' => 15,
];

if (isset($body['age'])) {
 if ($body['age'] === null) {
 echo 'Age was empty.';
 } else {
 echo 'The age you entered was ' . $body['age'];
 }
} else {
 echo 'No age given.';
}

If the body array is empty, the output will be 'No age given.'

If age is set to 15, the output will be 'The age you entered was 15'.

Now, when setting the age to null, you might expect the output to be 'Age was empty'. However, isset has a weird trait, that for array indices where the value is null, it will return false. This mistake may lead to some bugs, and if you explicitly want to check whether an array has a key, use array_key_exists.

SQL injection 💉

This mistake is one that's only applicable when you use a connection to an SQL database in your code, but since most PHP developers use (My)SQL, I think this is an important one to include.

Say a user enters their email address and you want to check if they are already registered.

$email = $_GET['email'];
$query = 'SELECT count(*) FROM users WHERE email = "' . $email . '"';
$userHasAccount = execute($query);

if ($userHasAccount) {
 echo 'You already have an account!';
} else {
 echo 'You do not have an account yet.';
}

If I now make a request where the query parameter email is set to "; TRUNCATE users; SELECT * FROM users WHERE email = ", the query that will be executed on the database is

SELECT count(*) FROM users WHERE email = ""; TRUNCATE users; SELECT * FROM users WHERE email = ""

Any malicious user is now able to delete your entire user database 😱

This is where prepared statements come into play. They make sure that any parameters passed in, are treated as strings in its entirety. To learn about how they work, you can read more in PHP's documentation. https://www.php.net/manual/en/pdo.prepare.php

Passing by value vs. by reference 🙃

$user = 'Thijs';
$age = 17;
echo calculateAgeNextYear($user, $age);
echo 'But today, you are ' . $age;

function calculateAgeNextYear(string $user, int &$age): string {
 echo 'Next year, you\'ll be ' . ++$age;
}

See the above code? It seems straightforward, right? You would expect the output to be:

Next year, you'll be 18
But today, you are 17

However, it'll tell you that today, you are 18. This happens because the $age parameter was passed by reference (see the &), instead of by value. By incrementing the $age inside of the calculateAgeNextYear, you actually changed the value of the $age outside the scope of the function.

At bunq, we never do this. In 9 out of 10 cases this leads to unwanted bugs which can be quite painful to debug. We always use objects, which are always passed by reference. However, these objects are always immutable. If you call a method on these objects, like $age->add(1) the value of the internal number is never changed; you'll instead be returned a new instance of Number.

Overengineering 🚀

This one isn't necessarily specific to PHP, but applicable nonetheless. Developers (and especially highly educated ones) tend to look for the absolute best solution out there. One that is very solid, scalable and highly flexible to be used in many situations. However, the cold hard truth is usually: you won't need it!

You're not building the next Google, and you're not going to get as much traffic as Amazon Web Services. Maybe your ambitions are to get somewhere close to that, but by the time you get there, this will be the least of your worries.

So, you don't need to spend 40% more of your time on abstracting something to interfaces so you can easily swap out the implementation for another one without changing much of your code. In reality, if you have to implement a new service, you're going to be spending time on writing a new implementation of this service anyway, and chances are you find out your interface doesn't work with the new service and needs to be refactored and you're already not better off anymore.

Hell, we're a bank, and while we take our code very seriously, we always strive to keep things simple. When the requirements change or a new service needs to be implemented, we refactor the code. Did you know that the first version of the bunq app didn't even have support for business accounts? When the time came and we wanted to accept business accounts, we refactored the code to allow for this and were done within 2 weeks.

It all boils down to KISS.

Premature optimization 📈

This one kind of ties in with the most previous mistake mentioned above, and also boils down to keeping things simple and not having to worry about scale. Many programmers worry about the usage of two nested `foreach` loops, when an `array_reduce` would have been more performant. You're talking about microseconds of difference, while all this time you have a 2MB header image on your home page that takes 2,5 seconds to load.

Again, we have a pretty high amount of network on our backend, but we don't get down to the nitty-gritty details of using one class instead of two because it would use less memory or be faster. We always prefer code quality and clarity over small performance differences. When something actually becomes a performance bottleneck, we take a look at how to solve it at that moment.

Instead of small incremental performance improvements, we take measures that affect the entire platform at once. So, when we saw a lot of queries being repeated during the same request, we built a database caching layer that would cache each query performed and store the result in memory. The next time this query is requested, the result is instead served from memory. When we did this (a couple of months after launching), we more than halved our average request duration.

Again, keep it simple. You're not Google, you'll be fine. And once you actually see it becomes a problem, you can fix it then.

Now get out there and code!

We hope these tips will help you along your way. If you think the way we work and code is great then come and join us build the Bank of The Free: we're hiring!