Skip to content

Instantly share code, notes, and snippets.

@Xerkus
Last active November 2, 2017 12:29
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Xerkus/1f4fad126e4c2fff57675b8f9185b5f0 to your computer and use it in GitHub Desktop.
Save Xerkus/1f4fad126e4c2fff57675b8f9185b5f0 to your computer and use it in GitHub Desktop.
<?php
namespace SpeckCatalog\Controller;
use Zend\Mvc\Controller\AbstractActionController;
use Zend\View\Model\ViewModel;
class ProductController extends AbstractActionController
{
protected $services = array(
'product' => 'speckcatalog_product_service',
'product_uom' => 'speckcatalog_product_uom_service',
'builder' => 'speckcatalog_builder_product_service',
'configure_buy' => 'speckcatalog_configure_buy_service',
'cart' => 'catalog_cart_service',
'renderer' => 'zendviewrendererphprenderer',
'option' => 'speckcatalog_option_service',
'product_image' => 'speckcatalog_product_image_service',
);
public function getService($name)
{
if (!array_key_exists($name, $this->services)) {
throw new \Exception('invalid service name');
}
if (is_string($this->services[$name])) {
$this->services[$name] = $this->getServiceLocator()->get($this->services[$name]);
}
return $this->services[$name];
}
public function indexAction()
{
$cartItemId = $this->params('cartItemId');
$cartService = $this->getService('cart');
$product = $this->getService('product')
->setEnabledOnly(true)
->getFullProduct($this->params('id'));
if (!$product) {
throw new \Exception('no product for that id');
}
$this->layout()->crumbs = $this->getService('product')->getCrumbs($product);
$vars = array(
'product' => $product,
'editingCart' => ($cartItemId ? true : false),
'cartItem' => ($cartItemId ? $cartService->findItemById($cartItemId) : false),
);
return new ViewModel($vars);
}
public function imagesAction()
{
$productId = $this->params('id');
$images = $this->getService('product_image')->getImages('product', $productId);
return new ViewModel(array('images' => $images));
}
public function getViewHelper($helperName)
{
return $this->getServiceLocator()->get('viewhelpermanager')->get($helperName);
}
public function uomsPartialAction()
{
$post = $this->params()->fromPost();
$pid = $post['product_id'];
$builderPid = isset($post['builder_product_id']) ? $post['builder_product_id'] : null;
$helper = $this->getViewHelper('speckCatalogUomsToCart');
$content = $helper->__invoke($pid, $builderPid);
return $this->getResponse()->setContent($content);
}
public function optionsPartialAction()
{
$postParams = $this->params()->fromPost();
$productId = $postParams['product_id'];
$options = $this->getService('option')->getByProductId($productId, true, true);
$renderer = $this->getService('renderer');
foreach ($options as $option) {
$vars = array('option' => $option);
$html .= $renderer->render('/catalog/product/option', $vars);
}
return $html;
}
}
<?php
namespace SpeckCatalog\Controller;
use Zend\Mvc\Controller\AbstractActionController;
use Zend\View\Model\ViewModel;
class ProductController extends AbstractActionController
{
private $productService;
private $cartService;
public function __construct(CartService $cartService, ProductService $productService)
{
$this->cartService = $cartService;
$this->productService = $productService;
}
public function indexAction()
{
$cartItemId = $this->params('cartItemId');
$product = $this->productService
->setEnabledOnly(true)
->getFullProduct($this->params('id'));
if (!$product) {
throw new \Exception('no product for that id');
}
$this->layout()->crumbs = $this->productService->getCrumbs($product);
$vars = array(
'product' => $product,
'editingCart' => ($cartItemId ? true : false),
'cartItem' => ($cartItemId ? $this->cartService->findItemById($cartItemId) : false),
);
return new ViewModel($vars);
}
// other actions extracted into separate controllers as they do not share dependencies and responsibilities
}
@Xerkus
Copy link
Author

Xerkus commented Nov 2, 2017

Line
Big problem of using service locator pattern is that lookup is hardcoded. Here we can see attempt to mitigate that by introducing map of keys. It can be solved more elegantly by service locator decorator that will do the mapping, but then we will need to make factory for controller anyway, might just inject deps directly.
Need for this is completely eliminated with dependency injection

@Xerkus
Copy link
Author

Xerkus commented Nov 2, 2017

Line
Here we see service locator being used.
What it brings to the table:

  1. It hides dependencies. Controller consumers MUST know which dependencies are required and at which keys in the locator.
  2. Missing dependencies are causing errors at execution time rather than object creation time.
    • We are not sure if service exists
    • We are not sure if service is of expected type
    • We will get exceptions here if dependency couldn't be created by dic for some reason.
  3. There is implicit execution branching here. Complexity is MUCH higher than that of $cartService = $this->cart;, where cart property is guaranteed to be set in constructor and its type enforced.
  4. We can not make another instance with alternative dependencies without providing alternative service locator
  5. Controller still have to know how to lookup dependencies, even if implementation details are abstracted and hidden by service locator. This is extra responsibility.
  6. Service locator solves the issue where we do not want to hold direct references to the dependencies
    • When we want to transparently swap dependencies at runtime ( as opposed to object creation time)
    • Allow to unload dependencies from memory
    • Not really useful in php due its single request nature.

What it means for tests?

  1. test for indexAction have to be aware of dependencies, service locator and keys for dependecies
  2. indexAction have to be tested for requesting service with key catalog_cart_service in this case.
  3. indexAction have to be tested for failure in case of failed lookup
  4. indexAction have to be tested for receiving wrong dependency (that test will or will not fail here as there is no check)
  5. Lookup errors that could interrupt execution and leave inconsistent state are particularly hard to test for

Quite common approach in tests is to have common helper method that sets up service locator with all the services registered. When that locator is used in tests, it is impossible to tell what is used by the tested code. That leads to fragile tests and potentially concealed bugs. Dependency injection is explicit, it eliminates this possibility completely.

@Xerkus
Copy link
Author

Xerkus commented Nov 2, 2017

Line
Responsibility to lookup dependencies is moved out. Controller object is guaranteed to have all required dependencies.

All dependencies are recorded explicitly. Reduced code complexity leads to improved readability and maintainability.

@Xerkus
Copy link
Author

Xerkus commented Nov 2, 2017

Line
No failure point here anymore, code is simpler and as a result more reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment