Forbidden changes in the code or continuation of the repair history of one crane

This article is a continuation of a previously published article, which can be found here .

In the current article, I will pay more attention to how, in spite of the limitations that the backward compatibility policy introduces, do not compromise on the quality of the code. And perform continuous refactoring during any code changes, and not postpone refactoring until it is allowed to make backwards incompatible changes, since only continuous refactoring, which is done every time the code changes, leads to continuous improvement in the code design and application architecture, which leads to improved extensibility and support for the code as a whole.

Postponing refactoring for later leads to an increase in technical debt and the creation of tasks (user story) for refactoring, which do not have a business value for the product owner, and, accordingly, such tasks will not fall into the top of the product backlog.

Prohibited code changes and how to get around them

Remove class / interface

Instead of deleting a class or interface, we mark this entity with the @deprecated annotation. We also @deprecated all the methods of this entity as @deprecated in order for all IDEs to highlight correctly all deprecated methods.
')
The reason why the entity has become @deprecated must be indicated after the annotation. @see annotation should be used to recommend a new API, which should be used instead of outdated.

 /** * @deprecated because new api was introduced * @see \New\Api */ 

We cannot know in advance in which version of the product the @deprecated code will be removed, since our plans may change (we are Agile), we can only know from which version the code has ceased to be relevant.
Therefore, in order to inform third-party developers about our plans to change the public code, we add the since marker with the @deprecated annotation
As in the example:

 /** * @deprecated since 2.1.0 * @see \Magento\Framework\Model\ResourceModel\Db\AbstractDb::save() */ public function save() { // ... } 

This marker should also be set by a programmer who does not manually write the code, since he may not know which release or patch his code will be in. And an automated script that makes a pre-release build. Thus, the build tool will insert since for the new code that should be put into the current release.

Remove public or protected method

Instead of deleting a method that can serve as a public contract or a contract for inheritance, you need to parse the method as @deprecated . Thus it is necessary to save the contract method.

Adding a new method to a class or interface

Since Magento does not know how the interface will be used as an API or as an extension point (SPI) (read more on this in Part 1 ), adding a new method to the contract is also a reverse incompatible change (hereinafter BiC), because if the interface is used as SPI, i.e. third-party developers (hereinafter - 3PD) provide their implementation for it, and replace the implementation out of the box via the DI configuration. By introducing a new method into the entity contract, we will bring problems for the 3PD class, which has no implementation for the new contract.

In the case of a class - we can always fall into a name conflict, if someone inherits this class and extends its contract.

In this case, you need:

• Create new interface with new method
• The new interface may also contain other methods that were contained in the original class, if it is appropriate and leads to an increase in cohesion.
• In this case, the corresponding methods of the original class should be marked as @deprecated with the addition of the @see annotation, which will indicate the contract in the newly created interface.
• Old methods should proxy calls to the methods of the created interface instead of duplicating logic, as well as to ensure the consistency of data during the operation of plug-ins and events.
• If changes are made as part of the PATCH release, then the new interface cannot be marked as @api

Remove static functions

Adding parameters, including optional ones, to public methods

Assuming that 3PD can still use the Inheritance Based API and inherit Magento classes by expanding their contract, adding a parameter to a method can cause us to break the inheriting class, which also added an optional parameter to the original contract.

Instead of changing the code of the old method, we need to introduce a new interface in which a new method signature will be specified that meets our changed business requirements.
Further see the procedure described in paragraph “Adding a new method to a class or interface”.

Adding a parameter to the protected method

Save the method as is. Create a new method with a new signature, and mark the old method as @deprecated . If possible, create a new method as private.

Change the type of arguments taken by the method

• Mark the old method as @deprecated
• We introduce a new interface with a new method, whose argument type is changed to the desired one.
• In the @see annotation of the old method refer to the new
• The implementation of the old method must be changed in such a way that the received old parameter must be converted to the new format, after which the call is proxied to the newly created interface

Change the type of the return value method

This task could be solved by returning the class type or the inheritor interface from the existing method in the contract. But since Magento does not recommend using the Inheritance Based API for new code, we do not use this practice.

Change the type of exceptions thrown

* only if the new type of exceptional situation is not a subtype of the old contract.

Modified Design Contract

In order to add a new parameter to the constructor it is necessary to make the parameter optional and add it last to the list of accepted arguments.

In the body of the constructor, there must be a check whether the new dependency is transferred, and if the new dependency was not transferred (the value of the passed argument is null), extract the dependency using the static method Magento\Framework\App\ObjectionManager::getInstance() .

Example:

 class ExistingClass { /** * @var \New\Dependency\Interface $newDependency */ private $newDependency; public function __construct( \Old\Dependency\Intreface $oldDependency, $oldRequiredConstructorParameter, $oldOptinalConstructorParameter = null, \New\Dependency\Interface $newDependency = null ) { ... $this>newDependency = $newDependency ?: \Magento\Framework\App\ObjectManager::getInstance() ->get(\New\Dependency\Interface::class); ... } public function existingFunction() { // Existing functionality ... // Use $this->newDependency wherever the new dependency is needed ... } } 

Does the BC fix always look ugly (like a tap on the KDPV) because we are limited in so many things?

And how to refactor, when you can not delete old dependencies transferred to the constructor , but you can only add new ones . Thus, we are bringing Dependency Hell closer when classes accept a huge amount of external dependencies, violating SOLID principles.

First, it is necessary to strictly prohibit the suppression of errors in static tests, which indicate that the coupling for the newly created code is exceeded. Those. Such SuppressWarning should not be added after performing a bug fix or implementing a new functionality.

Coupling Between Objects Increase Refactoring and Dependency Hell Prevention

If we introduce a new valid dependency into a class, for example, in order to fix a bug, and after that we pass the threshold of the number of permissible external dependencies ( 13 ).

We have to:

• Save backward compatibility class, i.e. its Public and Protected interfaces. But refactoring should be performed, which will lead to the fact that parts of the class logic will be moved to separate entities - small specialized classes.
• The existing class in this case must fulfill the role of a fascade in order to ensure the correct operation of the existing uses of the methods on which the refactoring was performed.
• Old public / protected methods should be marked as @deprecated with an @see annotation for the newly created methods in the new class.
• All unused private class properties can be deleted.
• All unused protected class properties must be marked as @deprecated
• A type of a class property variable that is not protected in PHP DocBlock can be removed, so it will not be counted by PHPMD as an external dependency.
• Type hinting in the constructor for an unused dependency must be removed, so it will not be counted by PHPMD as an external dependency.
• @SuppressWarnings (PHPMD.UnusedFormalParameter) must be added to the constructor to prevent static checks for unused arguments passed to the method from being triggered

After performing the steps described above, our crane code will shine with new colors. And most importantly, the contract will remain the same as it was before the changes, and accordingly the customers will not notice these changes!