Outdated Code - Third Party Code

In the TDD community, there is a tip that says that we should not use mock objects for types that we do not own . I think this is good advice, and I try to follow it. Of course, there are people who say that we should not use mock objects at all. Regardless of what opinion you hold, the advice “not to imitate what is not yours” - also contains a hidden meaning. People often ignore him, seeing the word "mock" and falling into a rage.



This hidden meaning is that you need to create interfaces, clients, bridges, adapters between our application and the third-party code that we use. Whether we will create mock-objects of these interfaces in our tests is not so important. The important thing is that we create and use interfaces that better separate our code from third-party. A classic example of this in the PHP world is to create and use an HTTP client in our application that uses the Guzzle HTTP client instead of using Guzzle directly.



Why? Well, for starters, Guzzle has a much more powerful API than the one that your application needs (in most cases). Creating your own HTTP client that provides only the required set of Guzzle APIs will limit application developers to what they can do with this client. If the Guzzle API changes in the future, we will need to make changes in one place, instead of correcting its calls in the entire application in the hope that nothing will break. Two very good reasons, and I did not even mention the mock-objects!



I do not think that this is difficult to achieve. Third-party code usually lies in a separate folder of our application, often vendor/ or library/ . It is also located in a different namespace and has a different naming convention than that used in our application. Third-party code is fairly easy to identify and, with a small amount of discipline, we can make our application code less dependent on third-party parts.

')

What if we apply the same rules to obsolete code?

What if we look at our Legacy code, as well as the third-party one? This can be difficult to do, or even counterproductive, if the outdated code is used exclusively in the support mode, when we only fix bugs and adjust small parts of it a little. But if we are writing a new code that (re) uses obsolete, I believe that it is worth considering it in the same way as third-party code. At least in terms of new code.



If possible, the outdated and new code should be located in different folders and namespaces. Much time has passed since I last saw the system without autoloading, so this is quite doable. But instead of blindly using the Legacy code in the new code, what if we make interfaces for it and use them?



Outdated code is often full of "divine" objects that do too many things. They use a global state, have public properties or magical methods that give access to private properties as if they are public, have static methods that are simply very convenient to call anyone from anywhere. So this is the most convenient and led us to the situation in which we are.



Another, maybe even more serious problem with outdated code is that we are ready to change it, fix it, crack it because we do not consider it as third-party code. What do we do when we see a bug or want to add a new feature to a third-party code? We describe the problem and / or create a pull request. What we do not do is not go to the vendor/ folder and do not edit the code there. Why do we do this with outdated code? And then we cross our fingers and hope that nothing is broken.



Instead of blindly using the outdated code in the new code, let's try to write interfaces that will include only the required subset of the API of the old “divine” object. Let's say we have a User object in obsolete code that knows everything about everything. He knows how to change email and password, how to raise forum users to moderators, how to update public user profiles, sets notification settings, keeps himself in the database and much more.



src / Legacy / User.php

 <?php namespace Legacy; class User { public $email; public $password; public $role; public $name; public function promote($newRole) { $this->role = $newRole; } public function save() { db_layer::save($this); } } 

This is a rough example, but displays a problem: every property is public and can be easily changed to any value, we need to remember to explicitly call the save method after any change to save, etc.



Let's limit ourselves and forbid access to these public properties and try to guess how the outdated system works when increasing user rights:



src / LegacyBridge / Promoter.php

 <?php namespace LegacyBridge; interface Promoter { public function promoteTo(Role $role); } 

src / LegacyBridge / LegacyUserPromoter.php

 <?php namespace LegacyBridge; class LegacyUserPromoter implements Promoter { private $legacyUser; public function __construct(Legacy\User $user) { $this->legacyUser = $user; } public function promoteTo(Role $newRole) { $newRole = (string) $newRole; //  ,  $role     ?  ! $legacyRoles = [ Role::MODERATOR => 1, Role::MEMBER => 2, ]; $newLegacyRole = $legacyRoles[$newRole]; $this->legacyUser->promote($newLegacyRole); $this->legacyUser->save(); } } 

Now, when we want to enhance User rights in the new code, we use the LegacyBridge\Promoter interface, which deals with all the subtleties of raising a user in an outdated system.

Change Heritage Language

The interface for outdated code gives us the opportunity to improve the design of the system and can save us from possible naming errors that were made a long time ago. The process of changing a user's role from a moderator to a participant is not “promotion” (promotion), but rather “demotion” (reduction). Nobody prevents us from creating two interfaces for these different things, even if the outdated code looks the same.



src / LegacyBridge / Promoter.php

 <?php namespace LegacyBridge; interface Promoter { public function promoteTo(Role $role); } 

src / LegacyBridge / LegacyUserPromoter.php

 <?php namespace LegacyBridge; class LegacyUserPromoter implements Promoter { private $legacyUser; public function __construct(Legacy\User $user) { $this->legacyUser = $user; } public function promoteTo(Role $newRole) { if ($newRole->isMember()) { throw new \Exception("Can't promote to a member."); } $legacyMemberRole = 2; $this->legacyUser->promote($legacyMemberRole); $this->legacyUser->save(); } } 

src / LegacyBridge / Demoter.php

 <?php namespace LegacyBridge; interface Demoter { public function demoteTo(Role $role); } 

src / LegacyBridge / LegacyUserDemoter.php

 <?php namespace LegacyBridge; class LegacyUserDemoter implements Demoter { private $legacyUser; public function __construct(Legacy\User $user) { $this->legacyUser = $user; } public function demoteTo(Role $newRole) { if ($newRole->isModerator()) { throw new \Exception("Can't demote to a moderator."); } $legacyModeratorRole = 1; $this->legacyUser->promote($legacyModeratorRole); $this->legacyUser->save(); } } 

Not a big change, but the purpose of the code has become much clearer.



Now, the next time you need to call some methods from obsolete code, try to make an interface for them. It may not be feasible, it may be too expensive. I know that the static method of this “divine” object is really very simple to use and you can do the job much faster with it, but at least consider this option. You can just slightly improve the design of the new system you are creating.