未加星标

Getting Rid of static - Qafoo - PHP

字体大小 | |
[开发(php) 所属分类 开发(php) | 发布者 店小二03 | 时间 2017 | 作者 红领巾 ] 0人收藏点击收藏

By Kore Nordmann, first published at Tue, 10 Jan 2017 09:34:30 +0100

Download our free e-book "Crafting Quality Software" with a selection of the finest blog posts as PDF or EPub.


Getting Rid of static - Qafoo - PHP
Getting Rid of static

When people start (unit-)testing their code one of the worst problems to tackle are static calls. How can we refactor static calls out of an existing application without breaking the code and while producing new features? How can we get rid of this big test impediment?

The Problem

Illustrating the problem with example code is only partially possible the sheer amount of static calls found in real-world software is way too large. The problem is worse by at least a magnitude from everything you'll see in this blog post. But here goes an example anyways:

class UserService { /* … */ public static function getUser($userId) { $cacheKey = 'user-' . $userId; if (Cache::has($cacheKey)) { return Cache::get($cacheKey); } $userData = DB::query('SELECT * FROM user WHERE id = :id', ['id' => $userId]); $user = new User($userData); Cache::set($cacheKey, $user); return $user; } /* … */ }

Using the cache statically is only one common thing to do. Usually it is also the logger, the database and about any service. Probably it is even the UserService itself with calls like UserService::getUser(42) spread all over your code.

Today it is an established best-practice that we should test our code. People working with code like the one shown above know this and want to apply automated testing. It is also clear that the most desired testing method are unit tests. When writing new methods in the UserService : How can we test those? Or how can we test the existing methods in the UserService ?

The core problem when testing the UserService is that we cannot mock the Cache instance for testing. The original Cache class will always be called, thus we will always test the Cache class together with the UserService . This is, by definition, not an Unit Test any more. Depending on the used cache implementation this might also require a far more complex setup to run the tests. If the cache implementation directly caches into Redis for example, you need a working Redis server just to run the UserService tests.

But getting away from static calls isn't a thing you should do in a single refactoring step. In fact, migration must be smooth and longer running to align with business perspectives. To achieve this, we show you multiple steps in such a migration.

Step 1: Replaceable Singletons

The workaround for the primary testing problem is obvious and employed by many developers you can even find complete testing frameworks build on this approach, like the one from Laravel : Make the implementation behind the static calls replaceable. Laravel calls this "Facade", while the Facade Pattern actually is something different.

The idea is that the Cache class gets a setter for its actually used cache implementation, like:

class Cache { private static $implementation; public static function setImplementation(CacheImplementation $cache) { self::$implementation = $cache; } /* … */ }

In your test case you can now use something like this:

class UserServiceTest extends phpUnit_Framework_TestCase { public function testLoadUser() { // Could happen in setUp() $originalCache = Cache::getImplementation(); /* Set up mock */ Cache::setImplementation($cacheMock); /* Actual test setup */ /* Stimulus */ /* Assertion */ // Reset should happen in tearDown() Cache::setImplementation($originalCache); } }

Why is this still problematic?

Global side effects

The worst thing here is the global side effect of this change. Since $implementation has to be a static variable in the Cache class it is also changed for any other code, like future tests. To get atomicity of your tests you must also reset the cache implementation after the test again.

More complex test setup

If you would use dependency injection the test code would only consist of the commented out code and would not require the setting and re-setting of the cache implementation. This might look trivial in this code but there are usually more classes used.

Also tests should focus on readable code even more then any other code since they are often the starting point to understanding code for other developers. Anything messing unnecessarily with the test code can be considered bad.

API differences due to indirection

The APIs of the Cache class and the CacheImplementation class might be different which means that you have to mock the internal usage inside of the Cache class and not the usage inside of the UserService class. You must at least ensure that those APIs are the same. This means that your brain must keep more context which leaves less focus on the actual implementation and test.

This is a valid workaround introducing additional complexity because of global state and unnecessary indirection. The static calls allow you to skip dependency injection by introducing additional complexity while understanding the code and while testing the code. A trade-off I am not willing to accept as a migration step but not in the long run.

Step 2: Service Locator

Looking at the actual target of our refactorings, the UserService we desire looks like this:

class UserService { private $cache; private $database; public funtion __construct(CacheImplementation $cache, Database $database) { $this->cache = $cache; $this->database = $database; } /* … */ public function getUser($userId) { $cacheKey = 'user-' . $userId; if ($this->cache->has($cacheKey)) { return $this->cache->get($cacheKey); } $userData = $this->database->query('SELECT * FROM user WHERE id = :id', ['id' => $userId]); $user = new User($userData); $this->cache->set($cacheKey, $user); return $user; } /* … */ }

What changed? We migrated all static dependencies to dependency injection via Constructor Injection and removed the static keyword for the getUser() method (and all others).

This code is testable. We can inject mocks for the CacheImplementation and Database directly and will not have any global side effects affecting out test atomicity. There is no environment related test setup required any more. The method itself could be cleaner and easier to test, but this is not the point right now. One important remark: You can also call every static method dynamically. Thus we can just pass an instance of Cache or CacheImplementation and Database and there should not be any problems.

The problem is that the API of our UserService changed. If we had code earlier calling UserService::getUser(42) it will not work any more. A simple step to get this code working is introducing a static Service Locator as a workaround during refactoring:

class ServiceLocator { private static $userService = null; /* Same for cache(), database(), … */ public static function userService() { if (!self::$userService) { self::$userService = new UserService(self::cache(), self::database()); } return self::$userService; } }

The we need to migrate all calling code to call ServiceLocator::userService()->getUser(42) instead of UserService::getUser(42) . This refactoring is easy enough and can usually be done by Search & Replace: s/UserService::/ServiceLocator::userService()->/g

The code using the UserService is still not clean, but we cleaned up the UserService itself to use dependency injection and be testable. We are not done yet, but this already is a big step forward.

Until now all steps are quick to accomplish, but the next one will take time.

Step 3: Dependency Injection

Now we have the UserService in the desired state. We already changed the code using it to indirect static calls through the ServiceLocator . But we should eventually get rid of all the static access, including the static access to the ServiceLocator itself.

Problems with using a Service Locator are:

Still global side effects (when used statically)

If you test a class which itself accesses the Service Locator you have to replace all requested services with mock objects in the Service Locator. Since the Service Locator implementation uses static state it will still affect all future tests (without proper resetting logic). This breaks test atomicity again.

…, thus also: Complex test setup

Hidden dependencies

If a class has access to the Service Locator you can only understand its dependencies from reading its entire code. In a class using Constructor Injection we know all its (required) dependencies by just looking at the constructor signature.

A class which has access to the Service Locator can request any class anywhere in its code from it. How do we know what to mock in a test case or what setup to create if we want to use the class somewhere else? You'll have to read the entire code.

This is also true for Service Locators with dynamic access which are passed around. They can be another intermediate refactoring step but usually provides nothing of additional value.

Consequence: Pass the ``UserService`` instance to every class which needs access to it. Or phrased differently: Use Dependency Injection for all your code or at least all code which is supposed to be tested or re-used at some point.

This means that, in production, you'll have just one instance of the UserService which is passed to any class which needs to access the users. This seems like a lot of work and it really is. But most of the work can actually be taken over by even the simplest Dependency Injection Container:

class DependencyInjectionContainer { private $userService = null; /* Same for cache(), database(), … */ public function userService($dic) { if (!$this->userService) { $this->userService = new UserService($this->cache(), $this->database()); } return $this->userService; } public function userController($dic) { return new UserController($this->userService()); } }

OK, this looks really similar to the ServiceLocator class shown before only dropping all static . This is right. The difference between a Service Locator and a Dependency Injection Container is not the implementation but the usage:

The DependencyInjectionContainer is only used inside your index.php and not passed to any class. As an additional migration step you may use it inside your Service Locator until we migrated away from it entirely.

If you have a simple routing definition like with Silex you should use the Dependency Injection Container right there and nowhere else:

$app = new Silex\Application(); $dic = new DependencyInjectionContainer(); $app->get('/user/{id}', function ($id) use ($app, $dic) { return $dic->userController()->getAction($id); }); $app->run();

The Dependency Injection Container will now resolve all the required dependencies only when the route is called . You can even replace the code above by using simple Dependency Injection Containers like Pimple .

This is the last step to have only testable classes all using Dependency Injection. The last step is a lot of work because many classes must be adapted. But you achieved actually two goals by doing this:

Make everything testable

Extract application configuration

Which cache is used by the user service should not be the concern of the user service itself but is nothing but application configuration. Now the DependencyInjectionContainer contains all your application configuration (it may access parameter files or similar) and you configure the used cache implementation there and it will be "magically" used everywhere.

We can help you refactoring your existing code while keeping your application functional. We will ensure through workshops, refactorings and code reviews that your goals are met. Hire us for akick-off workshop.

Conclusion

Migrating away from static calls can be quite some work. This blog post showed you a migration strategy with functional software in every step. Consider static a code smell because of all the reasons mentioned in this post and migrate away from it eventually. Take your time.

本文开发(php)相关术语:php代码审计工具 php开发工程师 移动开发者大会 移动互联网开发 web开发工程师 软件开发流程 软件开发工程师

主题: PHPRedisLaravel
分页:12
转载请注明
本文标题:Getting Rid of static - Qafoo - PHP
本站链接:http://www.codesec.net/view/522940.html
分享请点击:


1.凡CodeSecTeam转载的文章,均出自其它媒体或其他官网介绍,目的在于传递更多的信息,并不代表本站赞同其观点和其真实性负责;
2.转载的文章仅代表原创作者观点,与本站无关。其原创性以及文中陈述文字和内容未经本站证实,本站对该文以及其中全部或者部分内容、文字的真实性、完整性、及时性,不作出任何保证或承若;
3.如本站转载稿涉及版权等问题,请作者及时联系本站,我们会及时处理。
登录后可拥有收藏文章、关注作者等权限...
技术大类 技术大类 | 开发(php) | 评论(0) | 阅读(32)