Beware of Dumb Objects

In some projects I often see systems which look like this:

class InvoiceFactory
{
    public function __construct($service);
    public function createObjects($param);
}

class Invoice
{
    public function __construct($array);
    public function createObjects();
}

class InvoiceProducts
{
    public function __construct($object);
}

class WarehouseInvoice
{
    public function __construct($invoice, $products);
}

// Classes are used like this:
$invoices = new InvoiceFactory($database);
// Load database rows by id.
$array = $invoices->fetchInvoices($someId);
// Create useable objects from those rows
$invoice = new Invoice($array);
// List of products
$productList = $invoice->getProducts();
// Usable list products.
$products = InvoiceProducts($productList);
// Convert those rows into a usable form.
$objectsDto = $products->fetchProducts();
// Convert one useable form into the one that we actually need.
$products = new WarehouseProducts($invoice, $products);
// Convert list of products to one we can use in a template
$yetAnotherDto = $products->listProducts();
// Actually use the object.
doSomething($yetAnotherDto[0]->actualProperty);

Here we’ve gone from a factory to an object to a list of objects and back to a single object again several times. This can go on and on through several iterations of tiny one-function stateless objects that just convert data.

In this example, the code never actually does anything until the objects are converted into primitive data because all the models only accept primitive data. It seems attractive at first that all the objects would be without dependencies and behave as simple services, however there are several problems with this:

  • usage code is long
  • all objects involved need to be understood in terms of how they accept and produce data in order to know what’s going on
  • difficult to include data from other services without performing more data wrangling
  • difficult to change the behaviour of primitive data since it’s all pre-computed
  • no common API for access to data derived from the service, every client has to build their own conversion
  • difficult to unit test correct usage because it all relies on integration

There seems to be a natural tendency to obsessively break objects down to their smallest possible form, especially around a single database table or similar technical object.

I think the example above would have been just as good with a single object of functions to perform these conversions.

When you find yourself implementing this kind of pattern, consider using just one root service model and smarter objects to access the data.

// Persistence and external service access.
class ObjectService
{
    public function __construct($service);
    public function fetchObjectById($id);
    public function fetchExtraObjectData($id);
}

// Containing value object.
class ObjectList
{
    public function __construct($service, $id);
    public function getActualObjects();
}

// Element value object.  This can still use the ObjectService.
class ActualObject 
{
    public function __construct($objectList, $id);
    public function getActualProperty();
}

// Declare dependencies.
$service = new ObjectService($database);
$objects = new ObjectList($service, $id);
// Load usable data.
$list->getActualObjects()[0]->getActualProperty();

This gives you a more flexible implementation where your data access concerns are dealt with by the smart value objects and the service concerns are dealt with the root object service. The maintainer does not need to know anything about the interactions between these objects, they only need to know the dependencies (which should be defined by the constructor interface).

The conclusions I draw are:

  1. don’t be scared of high-level value objects
  2. dependencies are fine if they mean client code needs to know less
  3. create new abstractions only when they remove work from client code
Advertisements
Beware of Dumb Objects

Event Based Actions to Remove Boilerplate Code

In my application I tend to have a large number of actions that fall into a set of very similar boilerplate structures. They are never close enough to use a standardised model framework and there are too many different structures to use fancy controller configuration. A useful pattern to deal with this is to use events to remove the boilerplate.

public function createSomeObjectAction() 
{
    $form = new SomeForm();
    $editor = new SomeEditor();
    $action = FormAction::create($this, $form);

    $action->onValid(function($context) use($editor) {
       $id = $editor->createSomeObject($context->getValidData());
       return $this->redirect()->toRoute('EditObject', array('id' => $id));
    });

    $action->onOther(function($context) {
       $view = new \Zend\ViewModel\ViewModel();
       $view->setVariable('form', $context->getForm());
       $view->setTemplate('app/generic/bootstrap-from');
       return $view;
    });

    return $action->run();
}

It’s useful to see if you can write controller actions this way without using any if statements. This makes it easy to establish integration tests which just trigger each of the events. You also don’t need to do any deep zend specific configuration like you would with rendering strategies or event listeners — it’s “just php”.

The alternative here is to implement some interface which has its own idea of the onValid and onOther functions and passing this to some strategy.

public function createSomeObjectAction() 
{
    $editor = new SomeEditor();
    $model = new SomeObjectActionModel();
    $model->setEditor($editor);
    return ActionModelRunner::create($model)->run();
}

While this makes the action a lot shorter, it means that the maintainer now has to understand more APIs: the controller action, the action model interface, the action model, and the action model runner. With the event based system you only really need to understand the controller action and the FormAction. The functionality is right in front of you.

I find in general that reducing the number of APIs that the maintainer has to deal with is usually a good idea up to the point where the implementation complexity is too hard to understand in one go. These actions are basically just chaining function calls together — no if statements — so abstracting things does not have any benefit.

Another use case is to deal with errors as a structure:

public function createSomeObjectAction() 
{
    return $this->handleErrors(function() {
        $model = new SomeModel();
        $model->doSomethingFailable();
        return ViewModel();
    });
}

private function handleErrors($action) 
{
    $handler = new ApiErorrHandlerAction($action);
    $handler->on('SpecificException', function($handler, $ex) {
        return $handler->respondWithTemporaryError($ex->getMessage());
    });
    return $handler->run();
}

This is very useful when dealing with APIs which can throw errors at any point and reduces your need for the same boilerplate try-catch in every single action. Instead your intention is shown by the structure of the action.

The drawback is that your generic action needs to be somewhat understood by the reader. This is not too bad in the case of the FormAction example, but it’s more of a problem when you write something like a PaginatedListingAction or a CreateOrEditAction which will abstract dealing with your model code. It’s important to avoid making things so abstract and deeply layered that the maintainer can’t understand how the alter the behaviour of the system.

That said, if you have a lot of actions with the same structure, this technique of declaring behaviour as events can be more understandable and more flexible than a highly engineered framework based on model interfaces.

Event Based Actions to Remove Boilerplate Code