DEV Community: Michał Romańczuk

Equal or identical. How to compare variables?

Michał Romańczuk — Wed, 27 Jul 2022 05:50:13 +0000

== or ===? How many equal signs to put up so that it is correct and that nobody in the code review has a problem with it? Why is it so tricky in PHP?

Type Juggling

In PHP, the type of a variable is defined by how it was used*. Depending on what we have assigned to variable, it becomes that type¹. What does it mean?

(* From PHP 7 we can define types of method arguments, from 7.4 it is possible to define types of class properties!²)

$foo = "1";  // $foo is string
$foo += 0;   // $foo is integer
$foo *= 1.5; // $foo is float

In example above, string "1" has been assigned to the variable, so variable is string type, then integer 0 was added to it so it changes its type to integer. After that, let's multiply the integer variable by float 1.5. The variable is now float type.

This example shows how automatic conversion is performed by operators, in this case adding and multiplying. It is enough that one of the sides of the action is of the float type, then both will be treated as float and the result is also going to be of this type. The case is similar with integer although float is "stronger" and in the case of integer + float both sides will be treated as float.

Unequal equal

Let's get to equality, first look at the php documentation³ :

Equal: $a == $b, TRUE if $a is equal to $b after type juggling.

Sounds trivial and looks natural. Type juggling results in that, there is no need to think too much about what we compare, all is done automatically... ;) but is it?

Time for a quiz, will the following conditions be met?

0 == "eleven"
0 == "wtf-1.3e3"
1 == "\n\r1\t"
10 == "10 - 10"
"-1300" == "-1.3e3"
9223372036854775807 == "9223372036854775811"
"1.00000000000000001" == "1.00000000000000002"

Now attention, drums... in each of above cases it will be true. What just happened here? The, so far, highly intuitive comparison operator has now behaved in a manner rather unnatural to the eye.

How does the comparison operator actually work? To find out, it's best to look at how it was implemented⁴. However, this is not a simple reading, because the code is very complicated there. In short, the operation of == depends on the type of its arguments. Pairs of argument types are defined. If a given pair is supported (it is known how to compare it), the result of the comparison is returned, but if the given pair of types is not supported, the arguments are cast to another type (Type Juggling) and are only then compared.
In most cases this works in a predictable manner, but there are pairs of types that can surprise.

numeric value - string with a number and something extra

When string is compared with integer or float, then the string is cast to a numeric value, but this casting can sometimes be surprising.

10 == "10 - 10"; // true

In this case, the value of string is cast on to the integer, if string starts with a number, then whatever goes after that number is ignored. In this case "10 - 10" will not be the result of subtraction, so not 0 but 10.

numeric value - string with a non-number at the beginning

0 == "Lorem ipsum dolor sit 0"; // true

If string does not start with a number, then along with a numeric value it will always be cast on to 0.

numeric value - string with a big number

9223372036854775807 == "9223372036854775811"; // true

The integer from this comparison is the maximum integer value - PHP_INT_MAX (on 64-bit platforms), the string on the other hand includes a number slightly higher, just at 4 ;). In this case the integer will be cast to float, so the string eventually also to float. Such is the nature of float.

So never trust floating number results to the last digit, and do not compare floating point numbers directly for equality.

It's a warning from PHP documentation⁵ so it's better not to compare float values directly, but use dedicated functions:

numeric value - string with a number and white characters

1.0 == "\n\r1\t"; // true

White characters are ignored when casting to a numeric value. If you look at the example above you'll find one "\n\r1\t".

string - string

When comparing two string values it would be natural if it were true when they are the same ;) but what does it mean "the same".

"\n1" == "\r1"; // true
"1.0" == "\t1"; // true
"1e0" == "1";   // true
"-0.5e-2" == "-0.005";  // true
"1.00000000000000001" == "1.00000000000000009"; // true

"10" == "0xA";  // true  PHP < 7
                 // false PHP >= 7

When comparing string type arguments, at the beginning both arguments are cast to a numeric value. If it succeeds, they are compared as numerical values. Interestingly, PHP can do a lot of tricks when searching for a number in string⁶. Scientific notation or leading whitespace are not a problem, as well as hexadecimal notation up to version 7.0.

object - object

If it was too clear and predictable up to now, it works a little different when comparing objects.

class Example {
   public $property = 1;
}

$foo = new Example();
$bar = new Example();

$bar->property = 1.0;

$foo == $bar;    // true
$foo === $bar;   // false

Considering what I wrote earlier, everything in the example above is correct. The $foo and $bar variables are of the same type (Example class), their property ($property) in $foo is equal to 1 (integer) and in \$bar is equal to 1.0 (float). Thus, the result of the == comparison is true. Because the values match, types don't have to (integer vs. float). For === the result is false because the class property types do not match?

$baz = new Example();
$qux = new Example();

$baz == $qux;    // true
$baz === $qux;   // false

What happened here? Both variables are of the same type and their property is of the same value and of the same type, and still the result of === is false.
When comparing objects, identity (===) will return as true only if the same instance of the same class is compared⁷ . Yes. It is not enough for a variable's type (class) and all its properties to match, it must be same object. Comparison == will compare the object type and compare all its properties (also with ==).

So how should I compare?

In the examples I showed, it is clear that using the == comparison, sometimes may give a result different from what we expect. How to deal with this? Every time you write == (two equal signs), a red light should start flashing. We should always use the identity operator === (three equal signs), which also compares the type of the variable and does not cast anything before that. Well, almost always.

The only time we can use the equality == instead of identity operator === is when we do it intentionally and we have reasons for that!

Boy scout rule in 6 examples - the basic principle of web development

Michał Romańczuk — Mon, 20 Jun 2022 09:23:56 +0000

Not everyone was a scout in his youth. Most of programmers, however, have probably heard of the Boy Scout Rule in software development. But can you name a few of the activities that are part of that rule? When I started to explore this topic, looking for what exactly needs to be done to say "I just applied the boy scout rule", I only found one thing: "Leave your code better than you found it" - everyone quotes Uncle Bob. But what does it mean? What am I supposed to do exactly?

In scouting it is easier, everyone can see if the surroundings are clean, what can be cleaned to make it cleaner, but in the code? I will try to show you a few examples from my daily work where I try to stick to this principle.

Most of us use some IDE, they are getting smarter and in many cases, they keep the code clean, suggest better solutions, "clean up" automatically with one click. However, it's worth keeping an eye on what's happening in the code and not rely entirely on the IDE.

The campground, not an entire forest!

I am allergic to multiple blank lines. When doing a code review, I also pay attention to what the code looks like. The code must be readable, multiple blank lines, in my opinion, are disturbing. So I sometimes leave a comment in CR like "one blank line is enough (everywhere)". One day my colleague got it a little bit differently than I meant. That merge request before my first review contained 7 changed files. What was my surprise when I saw it after applying the changes - 350 files changed… he removed multiple lines from the whole project. You can not do this, such changes introduce a huge disruption to the code, the MR becomes completely unreadable.

You can't go overboard with cleaning. “Leave the campground cleaner than you found it”, the campground, not an entire forest!

Code cleanup should be limited to the files we work with. If we add a new method to the class and we notice something that can be improved/cleaned up "next to" - this is the place where we should apply the Boy Scout Rule.

Examples in practice

a) deprecations

If you use a smart IDE, it probably detects deprecated methods. The IDE I'm using strikes out deprecated names. Below, you can see crossed-out annotations @ Route and @Method. Something like that is very noticeable, it's hard to miss it.

Inside the classes of these deprecated annotations, I found lines like this:

This is a very easy change to apply. They say what exactly we have to do. Just like in the screen below. A few seconds change and one thing less hurts the eyes.

It was an outdated annotation, now an example of outdated method. More crossed letters below. Method setData(). Just like a moment ago. It cannot be overlooked.

A good scout will not ignore it. It is like a plastic bottle on the ground in the campground, that needs to be cleaned up. So just check this method, maybe the author left a hint. And I did it:

Again, we have exactly what to replace.

And the last one. We're using some vendor class here that is deprecated.

Just open it:

And another quick change

Of course, if you are using a less wise IDE, looking for deprecations is not that easy. Nobody in their right mind opens every class and method they use to look for information about deprecations. We just use what we know, usually without much thought. Therefore, it is worth using tools that support our work.

b) unused code

The code changes very often, especially in the early stages of development. With changes, sometimes you miss to delete an unused import, variable, or method.

A smart IDE detects unused code, for example, turns unused private methods to gray. This also works for unused imports, unused private properties and parameters.

But you have to be careful, I never trust the IDE completely, it is better to double-check that deleting a piece of code won't break something. Of course, your code is fully test covered, so you can be sure nothing's broken ;).

It's not that easy with public methods, they don't turn gray automatically ;). You need to be more careful with them. When you stop using some public method, let's just check to see if it’s used somewhere else. If not and no one needs it anymore, let's get rid of it.

We should remove unused code. That's what we use version-control systems for, so we don't have to see "maybe it will be useful someday" unused code. (YAGNI)

c) naming

We have to name many things in our work. Variables, methods, classes, everything must have a name. What should a good name look like? Not too long, not too short, and clear to anyone who can see it. It is not easy at all. I believe that this is a key part of our work. A badly named variable can mess up a lot and add a lot of work to others who later have to get into our code. It should be understandable without thinking. After reading the name of the method, the names of its arguments, and what it returns, we should know exactly what it does, without looking inside.

When I see the name of the variable $a, $var, $arr or $array in the code review, I immediately get a fever.

And below is an example of a variable with the two-letter name "ac". To know what it will contain, you need to look where something is assigned, it is easy to find if it is a constructor, and not always it will be there. It is obvious to the author of this code at the time of writing that "ac" stands for "authorizationChecker". But not everyone will figure it out, even the author after a while may not be sure what "ac" meant.

A good scout, seeing such a variable, will quickly change its name to clean up a bit. For example, such a change is enough:

Don't save on letters or you'll pay for it later trying to understand what the abbreviations mean.

d) code formatting

The worst thing I have seen about improving code formatting is to use autoformat in the IDE all over the project directory ("not an entire forest"!). Let's clean around the place where we work, but not too far, one file or one method is enough. Someone who reviews the code should be able to focus on important things and not scroll through hundreds of non-essential changes.

"My formatting is the clearest and most readable" - every programmer. Everyone has their own taste and habits, so you need to agree on the details of code formatting in your team. You probably have your standards in the project you work in. However, there is one thing everyone should follow - the PSR-12 (If you missed, the PSR-2 is deprecated).

Let’s see some examples.

one-line IF.

Very unreadable:

Just add a few spaces and new lines

or a short IF syntax

multiple blank lines

I don't know why, but very common

Just be a good scout and remove unnecessary blank lines

strange indentation in the code.

Are you uncomfortable reading it too? When you see something like this in the code, you can easily do the right thing and correct the indentation.

all other things incompatible with PSR-12

Boy Scout is not looking for whom to blame, but clean code himself, leaving the code better than he found (DO NOT apply this rule in code review, such things must be eliminated immediately! ;))

e) complex conditions

How many times have you wasted time trying to understand a complicated if condition? For me - too much. Worse, with many conditions, it is often easy to get confused (if you have tests, you are a lucky scout)

I made up an example below. let's say it's a method that handle some response. It first checks if the response is valid, if so, does something with it.

It is not overly complicated to analyze, but it does take a moment to analyze what is being checked here.

A good scout will find a few things here that he can clean up so that others will enjoy reading the code. But the most important thing is to separate the entire condition into a separate method whose name will clearly say what the condition checks.

Of course, in this example condition, many other things can be improved ;). But the point here was that we took the time to understand what it checks, so let's make it easy for other people not to waste that time.

f) code duplication

Why do we find duplicate code in projects? If something works in one place, it will work in another ;). But there is one problem. If you want to change or correct something later, you must remember to do it in all places where you copied the code fragment. Mindless copy-pasting is unacceptable and no one should do it. But we often see that it happens, all the time. When a good scout sees duplicate code, he just clean it. The smart IDE notices that the code is duplicated, it should alert you.

Here is an example to show what I'm talking about

We have two methods here, even if you search, you won't notice the difference other than the function name.

The first method sends an email with some positive decision, and the second method sends an email with some negative decision. For some reason, instead of extracting common code, someone just copied it.

Then, you have the task to change the title of sent emails, and you noticed what it looks like. Now you have two options what to do. First one is to only change the title in both places. If it works, why not? Well, NO! A good scout would do more than that. He'd clean up that piece of code.

It doesn't take much time, and the code is a bit cleaner and easier to maintain.

g) what else?

Anything that will improve the readability of the code or its maintenance. You are not doing it for others, you are doing it for yourself in the future.
Doing more serious refacotring (eg. to ensure SOLID princinples) may not be a quick job. But all the examples shown above can be done in less than a minute. This is well invested time. It will pay off in the future.

Need further help?

Boy Scout rule is a good practice that helps to keep the technical debt low. But sometimes it is just not enough. If you are no longer able to cope with the code of your web application just contact us. We help our clients fight the technical debt in legacy PHP & Javascript applications.
__

Simple Docker setup for Symfony project

Michał Romańczuk — Fri, 15 Apr 2022 13:43:47 +0000

It is no longer the case at Accesto that we work on projects without Docker. Of course, projects are not born dockerized, someone has to do it. Sometimes I do. I am a PHP developer and I work the most with Symfony framework on a daily basis. In this article, I will present an example development Docker configuration for the Symfony project.

So let's use a Docker in a Symfony project.

When it comes to Docker setup, there is no one-size-fits-all solution. It always depends on particular project needs. But some good practices should be followed, for example, those described in the official Docker documentation

Let's imagine we take over an existing project, a simple one, PHP, Symfony, MySQL database, that's all. But there is no Docker. So instead of installing a local web server, the appropriate PHP version, the appropriate database engine, etc., the first thing to do is ... dockerize!

Why should I spend time on dockerizing first? Because I will do it once, and everyone working on this project will benefit from it. You can read more about why it is worth using the Docker in the article of our CEO Piotr.

I assume you already have Docker Engine and Docker Compose installed, if not, you should do so.

Now let's focus on what we need. This project is currently running on Apache, PHP 7.4 and MySQL 5.7.

Configure Docker using docker-compose.yml

I have added the docker-compose.yml file in the main project directory, and I defined our first container in it - the one with the database. File docker-compose.yml is used by (Docker) Compose to manage and run Docker applications.

At the beginning of the file, you can (from version v1.27.0 it is optional) define the version of the configuration. Different versions of Docker Engine + Docker Compose work with different versions of the configuration, which you can check here.

version: '3.8'

MySql database in Docker container

Let's start by defining the first service, the database. First, the name, I chose “db”:

version: '3.8'

services:
    db:

After that I had to define what db service is. For example, we can use the image option. As an image, I had to select the appropriate Docker image. We can use ready-made images available on the web for this. Most of the tools come with Docker images. And to search for these images, we can use the search engine.

In my case, the project uses MySql version 5.7 and such an image is also available so I could just use its name:

version: '3.8'

services:
    db:
        image: mysql:5.7

As per the documentation for this image, some things in this container can be configured using environment variables.

One variable is mandatory, this is MYSQL_ROOT_PASSWORD. As you might guess, this is the MySql root superuser password. But there are a few other variables that are helpful, I used the following: MYSQL_DATABASE, a database with this name will be created during image startup. If we define MYSQL_USER, MYSQL_PASSWORD, a user with this name and password will be created, and he will have granted superuser access to the database defined in MYSQL_DATABASE.

version: '3.8'

services:
    db:
        image: mysql:5.7
        environment:
            - MYSQL_ROOT_PASSWORD=somerootpass
            - MYSQL_PASSWORD=somepass
            - MYSQL_DATABASE=dockerizeme_db
            - MYSQL_USER=someuser

Symfony Docker image

But there are no ready-made images for everything. For example, there is no ready image for a web server configured according to the needs of our application.

So while defining the container for our web server with PHP, I did not use the image option with the name of the ready image, but I used the build option with directory (.) that contains the file (Dockerfile) from which the image will be built. But about Dockerfile soon.

version: '3.8'

services:
    db:
        # cut for readability, see above
    web:
        build: .

Going further. It would be nice to add a basic Apache configuration. I added the docker directory in the main project directory, where I will keep files related to the Docker environment configuration. Then, in that directory, I added the apache.conf file. This file is a very basic configuration of a web server.

<VirtualHost *:80>
    DocumentRoot /var/www/public

    <Directory /var/www/public>
        AllowOverride None
        Order Allow,Deny
        Allow from All

        <IfModule mod_rewrite.c>
            RewriteEngine On
            RewriteCond %{REQUEST_FILENAME} !-f
            RewriteRule ^(.*)$ index.php [QSA,L]
        </IfModule>
    </Directory>
</VirtualHost>

Let's make our container accessible "from the outside". I used the ports option for this. Port 80 from the inside of the container will be exposed to the outside at port 8080.

version: '3.8'

services:
    db:
        # cut for readability, see above
    web:
        # cut for readability, see above
        ports:
            - 8080:80

Why didn't I put it on port 80 but 8080? Because we often have Apache running locally on port 80, so we would have a conflict and the Docker container wouldn't start.

Finally docker-compose.yml looks like this:

version: '3.8'

services:
    db:
        image: mysql:5.7
        environment:
            - MYSQL_ROOT_PASSWORD=somerootpass
            - MYSQL_PASSWORD=somepass
            - MYSQL_DATABASE=dockerizeme_db
            - MYSQL_USER=someuser
    web:
        build: .
        ports:
            - 8080:80

Dockerfile for Apache and PHP part

Now we need to take a step back. Let's go into the details of our web image. To define it, I added a Dockerfile file to the main project directory. Of course, we will use the ready-made public image as a starting point, and slightly adjust it to our needs. I used the php:7.4-apache image as a base.

At the beginning of Dockerfile, I defined the Docker image which is our base:

FROM php:7.4-apache

Then I enabled Apache mod_rewrite:

RUN a2enmod rewrite

After that I did the classic update, several packages installed such as libzip-dev (needed for zip which is needed for composer) git wget via apt-get install. And some extensions such as pdo mysqli pdo_mysql zip via docker-php-ext-install mechanism, more about that you can find in base image documentation. In the meantime, I also deleted files that are unnecessary in the container, they would only take up space.

RUN apt-get update \
  && apt-get install -y libzip-dev git wget --no-install-recommends \
  && apt-get clean \
  && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

RUN docker-php-ext-install pdo mysqli pdo_mysql zip;

The application uses Composer to manage dependencies in our application, so I installed in the container:

RUN wget https://getcomposer.org/download/2.0.9/composer.phar \ 
    && mv composer.phar /usr/bin/composer && chmod +x /usr/bin/composer

I copied the Apache configuration file to the appropriate place in the container, and our project files to /var/www in the container:

COPY docker/apache.conf /etc/apache2/sites-enabled/000-default.conf
COPY . /var/www

Instruction WORKDIR sets the working directory for other instructions, for example for the RUN command:

WORKDIR /var/www

And last command to run Apache in the container foreground:

CMD ["apache2-foreground"]

To sum up, we have such Dockerfile for web container:

FROM php:7.4-apache

RUN a2enmod rewrite

RUN apt-get update \
  && apt-get install -y libzip-dev git wget --no-install-recommends \
  && apt-get clean \
  && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

RUN docker-php-ext-install pdo mysqli pdo_mysql zip;

RUN wget https://getcomposer.org/download/2.0.9/composer.phar \
    && mv composer.phar /usr/bin/composer && chmod +x /usr/bin/composer

COPY docker/apache.conf /etc/apache2/sites-enabled/000-default.conf
COPY . /var/www

WORKDIR /var/www

CMD ["apache2-foreground"]

Is that all? As for the minimum, yes! Now if I run the docker-compose up -d in project directory in the console, the containers will be created and the whole project will start running.

But wait a minute. Let's see if it really works:

Of course not ;). Our project needs a few more steps. For example, what about composer install? Database migrations? Now we can describe these steps in the Readme of the project. It would be something like “enter the container (docker-compose exec web bash) and run composer install in the container, then run migrations (bin/console doc:mig:mig)”.

Improvements in startup a dockerized project

We can try to "automate" these steps, for example by using Entrypoint. Using Entrypoint is not the best solution, but it will facilitate development at this stage. Let's assume that I want to run every time the container is started:

composer install
new database migrations to be loaded
fresh fixtures (dummy/test data) in the database.

So I added entrypoint.sh in docker directory with a simple script to do it

#!/usr/bin/env bash

composer install -n
bin/console doc:mig:mig --no-interaction
bin/console doc:fix:load --no-interaction

exec "$@"

Then I had to slightly modify our Dockerfile.

FROM php:7.4-apache

# … cut for readability

COPY docker/apache.conf /etc/apache2/sites-enabled/000-default.conf
COPY docker/entrypoint.sh /entrypoint.sh

# … cut for readability

RUN chmod +x /entrypoint.sh

CMD ["apache2-foreground"]

ENTRYPOINT ["/entrypoint.sh"]

I COPY the new script file to the container and add executable permissions to that file. At the end of the Dockerfile, I defined that the file is Entrypoint

Now I had to rebuild the Docker image to apply these changes docker-compose build web, and restart the containers. Let's check our application:

It works! Now, anyone who downloads this project and has Docker can simply run it.

Can you also work with Docker on a Mac? You can read about it in the latest post by Krzysiek here :)

Docker is a powerful tool, I only showed you a minimal snippet that will allow you to start your project in a box. You can find more practical tips and advanced Docker optimization techniques in our free e-book Docker Deep Dive

Strangler Pattern in practice

Michał Romańczuk — Fri, 01 Apr 2022 08:58:54 +0000

This week I finally hit the delete button. I've been waiting for this moment for the last 3 years. Finally, I erased the legacy part of the codebase I was working on! Using a strangler pattern, we got to the point where the legacy SaaS application was completely replaced with a new shiny web application that everyone wants to work on and develop. But let’s start from the beginning.

Who likes working with legacy code? You may not believe it, but I do. It is always a challenge. Working on web application refactoring is usually archeology combined with graphology and patience training. Moreover, it is a job for the brave, let's be honest, how many legacy projects have tests? But if they do, these are usually red, and had been red a long time before we took over the project. If you change something in one place of a legacy code, you never know whether it won't break anything in another place that you don't know about, just because you haven't dug into it yet. Fixing a bug that needs to be fixed in the legacy code is similar to playing with dominoes. You fix one thing, and it causes you to discover another bug, and another, and another. And this way, from fixing one simple bug, it turns out you've changed 60 files. And when you finally deploy changes, you sweat while waiting for something to explode.

But don't worry, it is not the way we work with legacy code. At Accesto, we specialize in difficult cases. We usually take over the projects that others were afraid of. We like legacy, we usually live in peace with legacy code. But how is it even possible? How to deal with the legacy without going nuts?? Fortunately, there are a few ways to do it and maintain sanity. For example a Legacy can be tamed, Krzysiek showed it in his recent article. I'll show you another way.

So let's start from the beginning.

We took over a several-years old SaaS product that had already achieved business success in its market. But this success came with hidden costs, the cost of a technological debt (the costs of technical debt are described in detail by our CEO Piotr). It was a classic case, it was taking more and more time and effort to introduce new features, users were waiting a long time for bug fixes, and the entire website was working slower and slower day by day. Technical debt was so high that the previous developers quit their jobs.

The client, after a long search for someone who would like to help him, came to us.

Let's do a quick audit of what we got:

files with 2k+ lines of code? ✓
copy-paste driven development? ✓
hardcoded secret keys in code? ✓
PHP/HTML/CSS/SQL/JS in single file? ✓
code versioning with file names ✓
vulnerability to SQL injection, XSS, IDOR, etc.? ✓
FTP deployment? ✓
spaghetti code? (everywhere) ✓
and many more, it was not good, to sum up - a big ball of mud. ✓
SOLID princinples? forget it, that would require using at least some OOP ;-)

How did we approach it? To be honest, we didn't have much experience with the strangler pattern that time. We did have many successful SaaS refactoring projects behind us, but not with that approach. So where did we start?

Versioning

This legacy SaaS had a very peculiar way of versioning the code.

To make it funnier, the "latest version" was not always used everywhere. For example, the login page used "footer_v4.php" and the password reset page used "footer_v2.php", etc. So we couldn't just delete "older versions" that we thought were unused because there were "newer versions".

Copy-paste driven development + useful comments:

Anyone can make a bit of copy-paste. But "a bit" is crucial here. What we saw there deserved a master's degree in copy-pasting.

You won't find a significant difference between the two methods above. The entire system was based on the known methodology of "copy-pasting". How can such things be cleaned? I described an example in the article about "boy scout rule". But unfortunately, we couldn't put on rubber gloves and start cleaning, the strangler pattern is not about legacy refactoring. We tried to close legacy in a box and slowly replaced the old functions one by one until we could finally delete the "/legacy" directory. We tried not to touch the legacy code as long as we could. Unfortunately, it was not always possible, sometimes (actually very often at the beginning) the "urgent error" comes up. Then, as in the old times, we had to find and fix the bug (it is not easy in two-thousand-liners).

If you think leaving French comments in the code is okay, go to hell.

What options do we have?

keep going with legacy saas - add new features, fix reported bugs just like the developers who have just quit...
work on SaaS refactoring - slowly improve code quality, improving legacy php...
rewrite from scratch - favorite of all programmers but not business!

Some other ideas?

All of the above ways are hopeless. We wanted to try something different, something that will satisfy everyone, programmers, users and the business. And that is why we decided to use the strangler pattern.

What is the strangler pattern? In a nutshell, it’s incrementally replacing individual functionalities of the legacy system with new ones. Over time, nothing will be left of the legacy and the new system will replace all its functions.

Sounds trifle, doesn't it? So where to start?

Disclaimer: I would just like to point out that this is not a tutorial, this is a story of what we did some time ago. Could it be done better? For sure, and now we would have done some things differently. But we didn't go that wrong either.

Repository

We only got FTP access, so we need to civilize it. We've added code versioning (GIT). I was the unfortunate one who became the "author" of the entire legacy. I do not need to explain this, each of you knows that it is impossible to work without versioning. Simple, a new repository, git add . without hesitation git commit -m "clean legacy project" and git push. That's it.

Environment

To be able to work, we had to prepare a development environment. We put all the legacy code in a separate directory. Guess how it was named… yes you’re right "/legacy". After that, we deleted files that should not be in the repository like "/upload/*", something like a "cache", etc. Later we found a lot of useless files, but at this stage, we were not sure what was needed and what was not. Finally, we could create a simple Docker setup to run the project. Two containers on start, one for database and one for the legacy SaaS.

Facade

According to the strangler pattern - Gateway/Facade. We added the nginx reverse proxy, in the beginning, all traffic just went to the legacy container. The architecture looked like this:

Thus, we have taken the first step of the strangler pattern in practice.

New

Time for something enjoyable. We have to prepare a place for new features. We create a new project next to the legacy. The new, clean project, everyone likes them - the greenfield. We could choose what technologies we wanted, awesome. We chose what we are good at - a Symfony framework.

We started the easiest way - monorepo - next to the legacy we created the "/new" directory with Symfony and dockerize it. Now the structure of our project looked like this:

In the schema above, the size of the "new" and "legacy" boxes matters. All traffic was still routed across the gateway to the legacy. But we had a new container with clean Symfony that was connected to the same database as the legacy.

Let's stop here for a moment.

Database

We decided at that point to use the same database. Using the legacy database in the new part is actually an anti-pattern. We were aware of it, and despite that, we chose this solution. Why is it wrong? The boundary where the "New" and "Legacy" meet must be clear and transparent. When we use one database, we lose these boundaries. You don't see what is used by which part. It is not clear what can be changed and removed, what dependencies you have. The existing structure imposes a data model on you, so you are not able to create optimal solutions, you only use what you have. You improve the code by rewriting functionalities, but you are sometimes blocked by the existing data structure. Finally, you end up with a database where some of the tables are new, they are yours and you are proud of them, but they have relations to the old ugly ones, besides, there are many tables in the database that you don't even know what they're for. And worse, you don't know if you can delete them, because maybe something in legacy is using them, or maybe something in "new" is using it somewhere?

So, how should it be done in a perfect world?

Boundaries should be clearly delineated so it is best if the database is separate.

How can this be achieved? Synchronization.

Cyclic synchronization - it can be a script that synchronizes the databases cyclically, for example once every day.
Events - Legacy can dispatch events based on which New will update its copy of the data. But that requires legacy changes, and that can be tough.
DB Triggers - we can synchronize at the database level and use, for example use database triggers.
Transaction log - each transaction in the database leaves a log. We can use these logs and recreate them in a second database. For example, we can use Binlog + Kafka for this.

We summed up the pros and cons and decided to go single database. We had no experience with this pattern then, so we didn't know why it was wrong yet. As proof of the concept, we wanted to have a working project quickly so that we could test it in practice, and connecting directly to the Legacy base is quick and convenient. This has caused problems several times. Relationships and suboptimal model structure were the most problematic. We survived, the project survived it too, we finally managed to improve it all, but today we'd think twice or even three times before using one database.

Initial dump

Speaking of the database. Legacy as legacy did not use the full ORM (just something that pretended to be ORM), did not use migration and had no documentation.

All we had was production access to the database.

In order to be able to work in a development environment, it was necessary to prepare a safe database dump. It was a huge database. Over a hundred tables. A dump was needed to have some starting point for local development. First and foremost, anonymization, we had to go through all the tables and locate critical data that would be troublesome to come out. Another thing is data thinning. Out of several gigabytes of data, it was necessary to leave a maximum of tens of megabytes (because it will end up in the repository). Database relations were the problem, of course. This was a legacy, obviously not always the necessary foreign keys were added, so we had to be careful. Once the DB dump was prepared, we added it to the repository and prepared a script (in this case, it was phing) to load it into the database on the project setup.

Migrations

To manage database changes in a more predictable way, we decided to use Doctrine Migrations. But to be able to use migrations, we needed to have tables mapped to objects (ORM). I don't know if anyone would be able to map manually hundreds of tables to mapped objects without bugs. Luckily we didn't have to, Doctrine has a script ready for this (reverse engineering - yes, we started with SF 2.8 - it was already some time ago).

If you think that after that I ran the command that generates the migration and it was empty (it should be because we didn't want to change anything at this point, only map to objects), you are wrong. This script can't handle everything. It cannot cope with some data types (eg. tinyint is mapped to type boolean - regardless of length), some default values (eg. "0000-00-00 00:00:00" in datetime), and sometimes with relations (eg. stubbornly tries to rename foreign keys). So initially, such a migration, although it should be empty, contained several dozen changes that we did not make, it was necessary to review and decide what to do with them. We had to get used to some of them in the initial phase, if someone did not write the migration himself but generated them, he had to start by removing those changes that he did not make. And if someone forgot, it was remembered in the code review.

From now on, each change in the database will be implemented using Doctrine Migrations.

Tests

We decided to protect ourselves and write tests for basic business paths. It was not that simple, the project had no documentation, there were so many unused files that no one was able to tell what functionality it had at all. I dare to assume that most of the files in the "/legacy" directory weren't used at all. Even our client did not know about many of the existing functionalities in his system.

At the beginning, there was not much time for testing, it was practically no time for that. Tests are not what the client came for, he expected bug fixes and new functionalities, not days spent writing tests. It was just for us, for a quiet job.

So what to test? We decided to test what the client recognized as key functionalities. For the client, most of the functionality is crucial, we had to use our intuition. Of course, no one is able to write tests for an unknown existing system that will cover everything, having no budget for it. We used PHPUnit. We have also added smoke tests (does it return "200"?) to most of the subpages that we managed to find by clicking.

Secrets

What's next? What most people don't like to do, some urgent clean up the legacy. I will show an example, a database connection configuration:

As you can see, many "secrets" have been hardcoded. And the way it was done, it cried out to heaven for vengeance.

We decided to change this way of configuration to environment variables. There were many secrets in the system, often hidden among files with several thousand lines, defined wherever they were needed, they had to be found and replaced with environment variables. It's not a pleasant job, but it has to be done.

Security issues

The next step was to secure critical security issues. Legacy code was in poor shape from that point of view. Inserting URL parameters straight into SQL queries? Of course. I think I could write an article "Top 20 OWASP Vulnerabilities" and illustrate each subsection with examples we found in the code. Yes, it was that bad. We spent a lot of time finding and patching security issues.

When we told the client about the scale of the problem, we managed to persuade him to perform penetration tests by an external company that specializes in it. The several dozen pages report we received was very helpful.

My knowledge of security has grown significantly during this time. It hit me so much that later, for fun, I did "pentests" (it's easy when you have access to the code :)) of our other projects, upsetting my colleagues from other teams.

Continuous integration / Continuous delivery

As we are working on this project on Gitlab, you probably won't be surprised when I say that we used Gitlab CI. Here it was classic, we've added a few steps for build after commit, checking tests etc. Of course, most of CI's scripts were defined for the new part (which was still not doing anything at this point). After all, we will not run static legacy code analysis like phpstan, php-cs-fixer, phpmd etc, because the build would never pass, legacy php was a mess.

The docker images ready for deployment were built on the pushed git tag. We used the Rancher tool for test/stage environments.

We also used Capistrano for RC and production deployment. No more FTP deployment. What has it given to us? The possibility of quick rollback, speed, and certainty of deployed changes.

What about session and users?

As "new" and "legacy" are two separate docker containers, we had to choose some way to have information about the logged in user in the "new" part.

The first solution was a separate memcached container with users session that was shared between "new" and "legacy".

The legacy php part did not need to be changed:

The logged in user's username was already stored in the session (I am also puzzled by this comment "if true"). All we had to do was get to it in the new part.

So we used Symfony Guard. It is a cool and easy-to-use authentication mechanism.

The new part, having access to the username of the logged in user, can thus simply authorize him.

No, it wasn't that simple ;). In the old part, there was a complicated access control system based on numbers (sic!). Users had numbers in the database, each number meant a different level of authorization. Of course, this wasn't documented anywhere, and we had to take the time to understand what number means what permissions and recreate that in the new part on Symfony user roles.

Seriously, I'm not kidding, in legacy application, it looked like this ("ctrl+f" results below):

First tickets

Of course, the first tasks were all about hotfixes. Before the client came to us, he did a little research, bounced off a few companies that did not want to work with legacy SaaS. So, the project was running without technical support for some time. When the client came to us, the queue of errors to be fixed was already long. This is unfortunately a job that is not a strangler pattern, It is just a simple SaaS refactoring.

But at this point, we had the whole development setup ready to work with.

The problems were like with any legacy application. It was necessary to patch urgent bugs and optimize the code in some parts. We mainly used Blackfire and MySQL Slow Query Log to monitor and look for the causes of problems.

First new feature

But aside from improving performance and reliability, competition never sleeps. When we fixed the most urgent bugs, we started working on the new functionality in parallel with fixing the other bugs in legacy code. It's finally time for what everyone likes. It's time to go wild on the greenfield, our new part.

There were a few things that limited us. The legacy had some layout. To make the whole system consistent, we had to recreate these styles and page structure in the new part. This had some consequences. For example, adding a new item to a menu had to be done in both parts. It became necessary to maintain two systems.

Okay, we had CSS and layout in the new part. We could start working. Could we? Yes! We decided to prefix the paths for the new part. So our Reverse Proxy started redirecting paths starting with "/app/" to the Symfony container.

The user logged in, entered the system dashboard, a new item "Some new feature" appeared in the menu. So far, everything has happened in the legacy. But when he clicked on that new item in the menu he was redirected to "/app/some-new-feature". Reverse Proxy recognized the prefix "/app/" and routes the request to the new part of the system. We have it! Strangler pattern in practice.

Getting rid of legacy

In each iteration, we tried to replace some legacy. feature with a new one. We wanted to change the size proportions of the boxes new and legacy below with every release and make the box legacy finally disappear and we will be left with new.

For example, logging into the system was one of the first. Remember how it worked so far? The user logged in in legacy, his username was saved in the session (memcached) and the username in the new part was taken from the session, after which the user could be authorized in both. Now we had to reverse this mechanism. By the way, we decided to stop using the memcached session and replace it with the JWT token passed in the cookie. This was also necessary later to be able to scale the app in the Cloud (sessionless authentication).

In functionalities, not as crucial as logging in we used feature flag and canary releasing. We didn't use any fancy tools for the feature flags, simple bool in the database, feature enabled or not. Switching functionality from legacy to new sometimes involved migrating data from old legacy tables to the new database structure, so we did this for specific groups of users. After that, we switched the feature flag for those users. For users, this usually revealed a slightly refreshed layout of the old functionality. If everything was fine, we were gradually migrating data to subsequent users until everyone was using the new feature, and we could remove the legacy code and that feature flag at all.

Do you need to rewrite EVERY legacy functionality?

Of course not. But the client never wants to get rid of any functionality. So we used the user behavior tracking tools. And it was an enlightening experience, especially for the client. It turned out that some functionalities that the client considered to be one of the most important ones, were completely unused by users! Why waste energy and money on parts of the system that are not used by anyone?

By using the feature flags, we were gradually hiding some unused legacy functionalities from users. If nobody complained, we just removed them.

Scalability

Sometimes a product becomes popular. Traffic is increasing, we are opening up to new markets. We have come to the point where we had to migrate to the cloud. We choose Google Cloud and Kubernetes. The fact that we used the docker from the beginning was helpful. We had sessionless authentication, filesystem abstraction, all infrastructure in docker, we were cloud-ready, so that's happened.

So here we are today.

This week I finally deleted the legacy directory and its docker container.

Seriously, it inspired me to write this article as a remembrance of this event. We are with a product that works fully on our principles. The day has come when our "new" has become "new legacy".

How's your Legacy?

Legacy is not that bad if you get it right. If you are still not comfortable with your Legacy, feel free to write to us, maybe we can help you.

Using PHPStan with Symfony - static analysis for better PHP code quality

Michał Romańczuk — Mon, 14 Mar 2022 07:49:03 +0000

Software developers are not robots, even after seven years of PHP programming experience, I do sometimes make bugs in the code. In the age of smart IDEs, it's getting harder and harder to make silly mistakes, most IDEs catch them very well, but it still happens.

To avoid errors in the code, we can use tools that detect them during static code analysis. What kinds of errors are detected by static code analyzers? Lots, such as calls to unknown classes/methods, number of method arguments, using undefined variables, calling methods on nullable types, types of arguments passed to methods, and much much more.

There are many tools for static PHP code analysis, but one of the most popular is PHPStan. It is possible that it is due to its ease of use, versatility and the possibility of using many extensions for example to Symfony, doctrine, elasticsearch, monolog, guzzle etc. I'll show you how to easily add PHPStan to the Symfony project.

How to install PHPStan in a Symfony framework?

It is easier when we create a new environment and have a new, greenfield, clean and fresh project. Then we do not init PHPStan into the "dirty" code, but we have the code analysed from the beginning. But of course that rarely happens. If you haven't used static code analysis tools before, you are probably adding it because you already encounter some problems with your code.

First step, install PHPStan via Composer

composer require --dev phpstan/phpstan

It will install PHPStan’s executable in the project bin directory. Depending on your Symfony framework version/configuration it will be vendor/bin or just bin.

Second step.. just run the PHPStan analyse command (note that it is not a Symfony console command):

bin/phpstan analyse src

where src is a directory name you want to check.

That's it, you can start correcting bugs :). If this is a new project, you'll probably see there are no errors… but, let's use this in a project I was working on a while ago, and PHPstan wasn't used in it.

How to use PHPStan in a Symfony project?

PHPStan can be run with few parameters. One of them is required, this is the directory to be checked.

The level parameter is very important - "Specifies the rule level to run.". Analyse command can be run with one from nine levels where level 0 is the loosest and level 8 is the strictest. Default level is 0. How do these levels differ? You can check it in documentation. If this is a new, empty project, and you are just starting to write code, it is worth using a higher level. Then you will maintain high standards from the very beginning. But if you are adding PHPStan to an existing project, it will probably be not easy to pass even the lowest level.

How the response look like? Let’s check the project I mentioned. I will start from default level 0.

So in the src directory PHPStan found 53 bugs. What types of errors are these?

Access to an undefined property $foo
Method foo() invoked with X parameter, Y required.
Class \Bar constructor invoked with X parameters, Y required.
Call to an undefined method bar()
Method \Bar::foo() should return string but return statement is missing.

Even though 53 seems like a lot, it is very easy to correct it because these are simple ones.

So let's try to raise the level, how many bugs will it find? I do not have good news:

level 2 - 629 errors

level 8 - 2695 errors

No customer will agree if you tell him, now for a few weeks I will correct all errors found by PHPStan. You need to approach it wisely.

How to configure PHPStan?

Another parameter in the analyse command is --configuration (-c). It’s a path to file with configuration in NEON format (very similar to YAML).

What can we configure there? A lot, you can find all the options in the documentation but, for me, the most important options are paths to be checked and excluded from check, level and ignoring errors.

When I look at the result for level 2, I can see that most of the errors are PHPDoc tag @param has invalid value ($foo): Unexpected token "$foo", expected type. Let's be honest, this is not a critical bug, I won't be fixing all PHPDoc tags right now. Let's say I'll fix it another time, and now I'd like to ignore it (but make sure to put it on my refactoring list to keep a track on the technical debt). For this, I'll use the ignore error option in my configuration file. We can use regular expressions there, so I will ignore all errors starting with "PHPDoc tag ..."

parameters:
    ignoreErrors:
        - '#PHPDoc tag .#'

When you add your configuration file, do not forget to add it as a parameter for the analyse command. Otherwise, PHPstan will look for a file with the default name phpstan.neon

bin/phpstan analyse -l 2 -c myPHPStanConfig.neon src

This way the number of bugs has dropped from 629 to 268. I didn't fix these bugs, I just silenced them, I will correct them later (yeah, someday ;)) because they are not critical. I allowed myself to silence a few other minor bugs:

parameters:
    ignoreErrors:
        - '#PHPDoc tag .#'
        - '#. typehint specified#'
        - '#. return statement is missing#'
        - '#Return typehint of method .#'
        - '#Call to an undefined method Psr\\Container\\ContainerInterface::getParameter\(\)#'
        - '#Call to an undefined method Doctrine\\ORM\\EntityRepository<.+>::find.+\(\).#'

So right now I have "only" 108 erros to check and fix. But I am still on level 2, how does it look like on level 8?

833 errors, it’s not good, but that's better than the initial 2695 errors. Fortunately, this post is not about correcting bugs.

If this was a legacy project and it is not our priority to correct the legacy files, we can exclude whole legacy directores.

parameters:
     excludePaths:
        - src/Utils/*

With this configuration, PHPStan will skip all files in the src/Utils/* directory

PHPStan extensions

By default, PHPStan does not understand the magic that Symfony and Doctrine do sometimes. We can use framework specific extensions that expand it’s knowledge.

How to install PHPStan Symfony extension?

First we need to start with generic PHPStan Extension Installer, this is a composer plugin that handles the automatic installation of new extensions.

Just run

composer require --dev phpstan/extension-installer

Than install PHPStan Symfony framework extension:

composer require --dev phpstan/phpstan-symfony

After that, we need to define in the configuration file the path to the file that describes our container. It’s a XML file generated in the cache/dev directory. Extension documentation has some example:

parameters:
    symfony:
        # container_xml_path: var/cache/dev/srcDevDebugProjectContainer.xml
        # or with Symfony 4.2+
        container_xml_path: var/cache/dev/srcApp_KernelDevDebugContainer.xml
        # or with Symfony 5+
        # container_xml_path: var/cache/dev/App_KernelDevDebugContainer.xml

How to install PHPStan Doctrine extension?

PHPStan Doctrine extension provides support for all Doctrine magic. It checks our DQLs, QueryBuilders, validates entities and relations, recognizes magic methods like findBy* etc.
We already have "PHPStan Extension Installer", so we just need to install extension:

composer require --dev phpstan/phpstan-doctrine

To use more advanced analysis like DQL validation we have to provide an object manager. It’s described in extension documentation. We need to create a file eg. tests/object-manager.php

use App\Kernel;

require __DIR__ . '/../config/bootstrap.php';
$kernel = new Kernel($_SERVER['APP_ENV'], (bool) $_SERVER['APP_DEBUG']);
$kernel->boot();
return $kernel->getContainer()->get('doctrine')->getManager();

and use it in PHPStan configuration:

parameters:
    doctrine:
        objectManagerLoader: tests/object-manager.php

So let's check my project after adding these extensions.

For level 2:

For level 8:

The number of bugs found increased for level 2 from 108 to 429, and for level 8 from 833 to 1166 errors. As you can see, adding extensions improves code validation efficiency.

Other PHPStan extensions

There are many other extensions, for example you can check PHPStan strict rules extension. From their documentation:

This repository contains additional rules that revolve around strictly and strongly typed code with no loose casting for those who want additional safety in extremely defensive programming

And that's exactly how it works. I added it to my project. The number of errors detected increased on level 2 from 429 to 684 and on level 8 from 1166 to 1459.

How to automate PHPStan in a Symfony project?

Of course, even if you start with a clean project and you have zero errors, if you had to make sure that you run PHPStan before each Commit, you would forget one time. Someone else might not use it at all and with time the number of errors would grow.

From time to time you would have to sit and fix all bugs. This is not good, the customer will not want to pay for it.

Therefore, PHPStan should be added to your CI (you probably have some?). You can add the PHPStan check run job to composer scripts or directly to CI configuration. If it detects an error, then the build will fail, and you will know with each push that you need to correct something.

It is worth thinking about it from the beginning of the project, already at the stage of the initial setup and dockerization of the Symfony project, which I described in the previous post: Simple Docker setup for Symfony project

Conclusions

Is it worth installing PHPStan in an existing project? Of course, but.. probably not at the highest level from the beginning. You will have to start at the lowest level and work your way up gradually.I also recommend using extensions that only increase PHPStan capabilities, and thus find more potential bugs.

PHPStan is a great tool that will help you to maintain better quality of your PHP code. But remember, neither PHPStan nor any otehr tool will substitute the proper design (eg. following SOLID in PHP), practices (like a Boy Scout rule) and simply a thoughtfull development.

If you are not coping with your code, because you have not started using PHPStan on time, you can contact us, we deal with difficult cases.

Solid PHP - SOLID principles in PHP

Michał Romańczuk — Fri, 25 Feb 2022 07:39:39 +0000

SOLID, this acronym was coined by Michael Feathers, it represents the five basic principles of object-oriented programming developed by Uncle Bob.

Most programmers probably know this acronym. But it seems to me that a minority can decode it.

Is it wrong? In my opinion, no, I think that writing clean and simple code is more important than knowing the theory. But don't completely ignore the theory. How else will you pass on knowledge to someone? How do you justify your code in a discussion in code review? You have to be based on theory and generally accepted standards.

But it's also good to know the basics of what clean and simple code looks like.
SOLID principles can be used in any object-oriented programming language. I work in Symfony on a daily basis, so I will show some principles in PHP.

So let's go through the five SOLID principles together.

Single responsibility principle (SPR)

I think this is the most famous rule (probably because it is the first and some people did not read on). But seriously, I think it is very important.

Uncle Bob describes it as “A class should have one, and only one, a reason to change”. What does it mean? For me, this sentence is not helpful :).

Other explanations say that a class, a function should do one thing.

But what is one thing? Is user registration one thing? Or maybe it's more things, because registrations include some other smaller things, password encryption, saving to the database, sending an e-mail.

Is sending an email one thing? After all, it consists of many steps such as preparing the e-mail content and subject, extracting the user's e-mail address and handling the response. Should we set up a separate class for each of these activities? How far do we go with this “single responsibility”?!

I think we need to use a different object-oriented programming principle to answer this question. In 1974, the "high cohesion and low coupling" principle was described for the first time in the article Structured Design in the IBM journal.

I will try to present it in simple to understand examples.

Cohesion determines how much a function or class is responsible for. An simple example here would be Bob and Alice, the cook's helpers. Alice is making desserts. She needs to make a sponge cake, cream, glaze, cut the fruit and put it all together. Each of these steps consists of several others. It's an example of low cohesion. Bob's job is to peel potatoes, nothing else, it’s an example of high cohesion. Your method/class should be like Bob, do one thing.

Coupling is about how easy it is to reuse a given module or class. Puzzles and Lego blocks are a good example for that. The puzzles are characterized by high coupling. One puzzle fits only in one place, it cannot be combined with other puzzles. The opposite of Lego bricks, they have a low coupling, they can be combined freely and each one can be used anywhere. Your code should be like Lego blocks, easy to use in different places.

The single responsibility principle should be used together with the "high cohesion and low coupling" principle. Both of these principles, in my opinion, try to say the same.

Now an example in PHP. Imagine a BlogPost class:

class BlogPost
{
    private Author $author;
    private string $title;
    private string $content;
    private \DateTime $date;

    // ..

    public function getData(): array
    {
        return [
            'author' => $this->author->fullName(),
            'title' => $this->title,
            'content' => $this->content,
            'timestamp' => $this->date->getTimestamp(),
        ];
    }

    public function printJson(): string
    {
        return json_encode($this->getData());
    }

    public function printHtml(): string
    {
        return `<article>
                    <h1>{$this->title}</h1>
                    <article>
                        <p>{$this->date->format('Y-m-d H:i:s')}</p>
                        <p>{$this->author->fullName()}</p>
                        <p>{$this->content}</p>
                    </article>
                </article>`;
    }
}

What's wrong here? The BlogPost class does too many things, and as we know it should do one thing. The main problem here is that it is responsible for printing to various formats, json, html and more if needed. So let's see how this could be improved.

We remove printing methods from the BlogPost class, the rest remains unchanged. And we're adding a new PrintableBlogPost interface. With a method that can print a blogpost.

interface PrintableBlogPost
{
    public function print(BlogPost $blogPost);
}

Now we can implement this interface in as many ways as we need:

class JsonBlogPostPrinter implements PrintableBlogPost
{
    public function print(BlogPost $blogPost) {
        return json_encode($blogPost->getData());
    }
}

class HtmlBlogPostPrinter implements PrintableBlogPost
{
    public function print(BlogPost $blogPost) {
        return `<article>
                    <h1>{$blogPost->getTitle()}</h1>
                    <article>
                        <p>{$blogPost->getDate()->format('Y-m-d H:i:s')}</p>
                        <p>{$blogPost->getAuthor()->fullName()}</p>
                        <p>{$blogPost->getContent()}</p>
                    </article>
                </article>`;
    }
}

You can see a whole example of bad and good implementation here

I've seen projects where classes only have one public method with a few lines of code (usually call to a different method from a different class). Completely illegible and terrible to maintain. In my opinion, this is an example of going too far.

To sum up. Your classes and methods shouldn't be responsible for a few things. But the point here is not to go to extremes and exude absolutely everything. Just to make them easy to understand, but they also have to be consistent. So that you don't have to read them cover to cover to understand what they are doing.

Open/closed principle (OCP)

Second, from SOLID principles. The general explanation is that “code should be open for extension, but closed for modification”. It is not obvious to me what this means in practice. Perhaps it is better explained by the consequence of not following this rule. Changing the declaration of a method may cause it to malfunction somewhere it is used. The main point is that the changes have to be backward compatible. Of course, it's best to write code that works perfectly from the beginning and you don't have to change it, but we don't live in a perfect world.

I will try to present it with some examples:

a) open/closed API

This will be an example of the open/closed principle not on a single class but on the entire API. It's a large SaaS, it is an accounting system written in PHP, Symfony Framework. Your API is used by several hundred customers who use it to issue invoices. Your API has a method to retrieve invoices as PDF. Let's say it is an endpoint like “GET /invoice/{id}/print”. Everything is fine, but one day customers demand the option to download CSV (everyone from business loves tables).

So you implement this capability quickly and change the endpoint from:

"GET /invoice/{id}/print"

"GET /invoice/{id}/{format}"

where the format can be PDF or CSV.

Now only hundreds of programmers using your API have to change how they download the report in PDF. Well, no, it shouldn't be done that way. How to do it correctly? Unfortunately, it is sometimes necessary to see potential problems and anticipate possible future changes. From the beginning, your endpoint did not follow the open/closed principle because it was not closed for modification. Your endpoint should assume that the needs of other formats may arise someday.

b) open/closed animals

Another example, a more classic one. Let's say we have several different animal classes:

class Dog
{
    public function bark(): string
    {
        return 'woof woof';
    }
}

class Duck
{
    public function quack(): string
    {
        return 'quack quack';
    }
}

class Fox
{
    public function whatDoesTheFoxSay(): string
    {
        return 'ring-ding-ding-ding-dingeringeding!, wa-pa-pa-pa-pa-pa-pow!';
    }
}

And a class that allows animals to communicate:

class Communication
{
    public function communicate($animal): string
    {
        switch (true) {
            case $animal instanceof Dog:
                return $animal->bark();
            case $animal instanceof Duck:
                return $animal->quack();
            case $animal instanceof Fox:
                return $animal->whatDoesTheFoxSay();
            default:
                throw new \InvalidArgumentException('Unknown animal');
        }
    }
}

Is the Communication class open for extension and closed for modification? To answer this question, we can ask it differently. Are we able to add a new animal class without changing the existing code? No. Adding a new animal class would necessitate the modification of the switch in the communicate() function. So what should our code look like to comply with our principle? Let's try to improve our classes a bit.

We can start by adding an interface Communicative, and using it in our classes.

interface Communicative
{
    public function speak(): string;
}

class Dog implements Communicative
{
    public function speak(): string
    {
        return 'woof woof';
    }
}

class Duck implements Communicative
{
    public function speak(): string
    {
        return 'quack quack';
    }
}

class Fox implements Communicative
{
    public function speak(): string
    {
        return 'ring-ding-ding-ding-dingeringeding!, Wa-pa-pa-pa-pa-pa-pow!';
    }
}

After that, we can change the Communication class so that it complies with the open/close principle.

class Communication
{
    public function communicate(Communicative $animal): string
    {
        return $animal->speak();
    }
}

How to code according to the opened/closed principle?

In code, it is worth using interfaces and sticking to them. However, if you need to change something, consider the decorator pattern.

A class or method should be small enough and have one specific task so that no future event can necessitate modification (single responsibility principle). But you also need to consider whether there may be a need for changes in the future, such as a new response format or an additional parameter, your code should be closed for modification.

Liskov substitution principle (LSP)

Substitution principle applies to well-designed class inheritance. The author of this principle is Barbara Liskov. The principle says that we can use any inheriting class in place of the base class. If we implement a subclass, we must also be able to use it instead of the main class. Otherwise, it means that inheritance has been implemented incorrectly.

There are some popular examples of the Liskov substitution principle in PHP:

a) rectangle-square

The first example. We already have a Rectangle PHP class. Now we're adding a Square PHP class that inherits the Rectangle class. Because every square is also a rectangle :). They have the same properties, height and width.

The height of the square is the same as the width. So, setHeight() and setWidth() will set both (what about single responsibility?) of these values:

class Square extends Rectangle
{
    public function setWidth(int $width): void { 
        $this->width = $width;
        $this->height = $width;
    }

    public function setHeight(int $height): void {
        $this->width = $height;
        $this->height = $height;
    }
}

Is that a good solution? Unfortunately, it does not follow the Liskov substitution principle. Let's say there is a test that computes the area of a rectangle, and it looks like this:

public function testCalculateArea()
{
    $shape = new Rectangle();
    $shape->setWidth(10);
    $shape->setHeight(2);

    $this->assertEquals($shape->calculateArea(), 20);

    $shape->setWidth(5);
    $this->assertEquals($shape->calculateArea(), 10);
}

According to the Liskov substitution principle, we should be able to replace the Rectangle class with the Square class. But if we replace it, it turns out that the test does not pass (100 != 20). Overriding the setWidth() and setHight() methods broke the Liskov substitution rule. We should not change how the parent class's methods work.

So what is the correct solution? Not every idea from "reality" should be implemented 1:1 in code. The Square class should not inherit from the Rectangle class. If both of these classes can have a computed area, let them implement a common interface, and not inherit one from the other since they are quite different.

You can see an example solution here

b) live duck vs toy duck

Imagine a living duck and a toy duck and their representations in the code (PHP classes). Both of these classes implement the TheDuck interface.

interface TheDuck
{
    public function swim(): void;
}

We also have a controller with the action swim().

class SomeController
{
    public function swim(): void
    {
        $this->releaseDucks([
            new LiveDuck(),
            new ToyDuck()
        ]);
    }

    private function releaseDucks(array $ducks): void
    {
        /** @var TheDuck $duck */
        foreach ($ducks as $duck) {
            $duck->swim();
        }
    }
}

But after calling this action ToyDuck doesn't swim. Why? Because to make it swim, you must first call the "turnOn()" method.

class ToyDuck implements TheDuck
{
    private bool $isTurnedOn = false;

    public function swim(): void 
    {
        if (!$this->isTurnedOn) {
            return;
        }

        // ...
    }
}

We could modify the controller action and add a condition that we call turnOn() on the ToyDuck instance before swim().

private function releaseDucks(array $ducks): void
{
    /** @var TheDuck $duck */
    foreach ($ducks as $duck) {
        if ($duck instanceof ToyDuck) {
            $duck->turnOn();
        }

        $duck->swim();
    }
}

It violates the Liskov substitution principle, because we should be able to use a subclass without knowing the object, so we cannot condition by subclasses (it also violates the open/close principle - because we need to change the implementation).

Handling a collection of objects of a given base class may not require checking whether the given object is an instance of subclass X and should be treated differently.

What should it look like correctly? Common interface for both of these ducks is not a good idea, their operation is completely different, even though we think they both work similarly because they are swimming, it is not.

c) ReadOnlyFile

And the last example. We have a File class with methods read() and write().

class File
{
    public function read()
    {
       // ...
    }

    public function write()
    {
       // ...
    }
}

We're adding a new class - ReadOnlyFile.

class ReadOnlyFile extends File
{
    public function write()
    {
        throw new ItsReadOnlyFileException();
    }
}

The ReadOnlyFile class inherits from the File class. In the ReadOnlyFile class, the write() method will throw an Exception, because you cannot write to a read-only file.

This is a poorly designed abstraction, the Liskov rule has been broken because we are unable to use the ReadOnlyFile class instead of File.

Interface segregation principle (ISP)

Uncle Bob introduced this principle when he collaborated with Xerox. They couldn't cope with the ever-long process of implementing changes to their code. The rule is: “No client should be forced to depend on methods it does not use”. The user of the interface should not be forced to rely on methods he does not use. We should not use “fat interfaces” that declare multiple methods if any of them could be left unused. Better to have a few dedicated small interfaces than one that is too general. It is also in line with the single responsibility principle.

So let's see a badly written code, not following the interface segregation principle. I present to you the Exportable, PHP Interface. An interface that allows you to export something to PDF and export something to CSV. We also have an Invoice and a CreditNote class.

interface Exportable
{
    public function getPDF();
    public function getCSV();
}

class Invoice implements Exportable
{
    public function getPDF() {
        // ...
    }
    public function getCSV() {
        // ...
    }
}

class CreditNote implements Exportable
{
    public function getPDF() {
        throw new \NotUsedFeatureException();
    }
    public function getCSV() {
        // ...
    }
}

We can download the Invoice in PDF and CSV. We can download a CSV of the CreditNote. But downloading the PDF of the CreditNote was a useless functionality and was not implemented (it’s throwing an exception right now).

We shouldn't force our interface implementations to implement methods they don't use. In the above case, we forced the CreditNote class to do so, it implements the getPDF() method even though it does not need it at all.

So how should it look to be good?

According to the interface segregation principle, we have to separate the interfaces. We divide Exportable, and create an interface ExportablePdf and create an interface ExportableCSV.

interface ExportablePdf
{
    public function getPDF();
}

interface ExportableCSV
{
    public function getCSV();
}

class Invoice implements ExportablePdf, ExportableCSV
{
    public function getPDF() {
        //
    }
    public function getCSV() {
        //
    }
}

class CreditNote implements ExportableCSV
{
    public function getCSV() {
        //
    }
}

This way, CreditNote no longer has to worry about implementing not used the getPDF() public function. If necessary in the future, just need to use a separate interface and implement it. As you can see here, specific interfaces are better.

The example about ReadOnlyFile related to the Liskov principle is also a good example for the Interface segregation principle. There, the File class has been doing too many things, it's better to have separate interfaces for each action.

That's interface segregation, easy.

Dependency inversion principle (DIP)

Last form SOLID principles, this rule is:

High-level modules should not import anything from low-level modules. Both should depend on abstractions (e.g., interfaces).
Abstractions should not depend on details. Details (concrete implementations) should depend on abstractions.

What does it mean? We should reduce dependencies to specific implementations, but rely on interfaces. If we make any change to the interface (it violates the open/close principle), this change necessitates changes in the implementations of this interface. But if we need to change a specific implementation, we probably don't need to change our interface.

To illustrate the problem, let's go over this PHP example.

class DatabaseLogger
{
    public function logError(string $message)
    {
        // ..
    }
}

Here we have a class that logs some information to the database. Now use this class.

class MailerService
{
    private DatabaseLogger $logger;

    public function __construct(DatabaseLogger $logger)
    {
        $this->logger = $logger;
    }

    public function sendEmail()
    {
        try {
            // ..
        } catch (SomeException $exception) {
            $this->logger->logError($exception->getMessage());
        }
    }
}

Here is the PHP class that sends e-mails, in case of an error, error details are logged to the database using the logger we have just seen above.

It breaks the principle of dependency inversion. Our e-mail sending service uses a specific logger implementation. What if we want to log information about errors to a file or to Sentry? We will have to change MailerService. This is not a flexible solution, such a replacement becomes problematic.

So what should it look like?

According to this principle, MailerService should rely on abstraction rather than detailed implementation. Therefore, we are adding the LoggerInterface interface.

interface LoggerInterface
{
    public function logError(string $message): void;
}

And we use it in our DatabaseLogger:

class DatabaseLogger implements LoggerInterface
{
   public function logError(string $message): void
   {
       // ..
   }
}

Now, we can take advantage of Symfony Dependency Injection.

class MailerService
{
    private LoggerInterface $logger;

    public function sendEmail()
    {
        try {
            // ..
        } catch (SomeException $exception) {
            $this->logger->logError($exception->getMessage());
        }
    }
}

In this way, we can freely replace the logs in the database with logs wherever we want, as long as the detailed implementation implements the LoggerInterface. This change will not require modifying MailerService, because it does not depend on it, it depends only on the interface.

SOLID

All these principles come together as one, they often overlap. It's nice when you know the theory like SOLID principles because it makes it easier to make good code. Then you also have strong arguments behind your code, for example in code review. All the rules are aimed at making the code easy to understand and maintain.

SOLID is one of the many good practices that help us write clean code. I've written about the Boy Scout Rule before. But that's not all, there are many other rules and standards to follow. Let me just mention them:

PSR (PHP Standards Recommendations) - PHP Framework Interop Group (PHP-FIG) is a group of people associated with the largest PHP projects who jointly develop PSR. I think every PHP programmer should know coding styles standards PSR-1 and PSR-12 (formerly PSR-2). You can find all the current sets of standards here
KISS (Keep It Simple Stupid) - Don't complicate the code. The code should be its documentation itself. Any new programmer on the team should be able to get into the project quickly.
DRY (Don’t Repeat Yourself) - Do not code using the Copy-Paste principle (there is no such rule). See that the same code repeats in several places? Extract code for a separate function.
YAGNI (You Aren’t Gonna Need It) - 17th-century German philosopher Johannes Clauberg formulated a principle called Occam's Razor (I was also surprised Ockham was not its author ;) ) “entities should not be multiplied beyond necessity". I think this sentence expresses the YAGNI principle well. We should not write code “for the future”. Such code is not needed at the moment.
GRASP (General Responsibility Assignment Software Patterns) - is a large set of rules about which I could write a separate article. These are the basic principles that we should follow when creating object design and responsibility assignments. It consists of: Information Expert, Controller, Creator, High Cohesion, Low Coupling, Pure Fabrication, Polymorphism, Protected Variations, Indirection.

Applying the SOLID principles in our daily work helps us not get into technical debt. What are the consequences of incurring technical debt, you can find out in the article written by our CEO Piotr.

If you have problems understanding your project. Write to us, we have experience in dealing with difficult cases and PHP refactoring.