Using a separate class in Symfony to handle a list of entities with controller injection

Question

In my Symfony (v2.7.9) project I have a set of entities which share some view functionality i.e being displayed in a table with some of their values as columns.

Following the separation of concerns principle, I've made a ListTable class which handles getting the table headers, rows and column values and I'm extending this for entities which need a little bit of extra functionality e.g ProductListTable

class ListTable extends AbstractLibrary {
    protected $items = [];
    protected $query;
    protected $controller;

    protected $tableKeys = [
        'name',
        'status',
    ];

    public function __construct( BaseController $controller ) {
        $this->controller = $controller;
        $this->query = $controller->request->query;
    }

    public function getTableKeys() {
        return $this->tableKeys;
    }

    public function getItems() {
        if ( empty( $this->items ) ) {
            foreach ( $this->controller->getResults() as $entity ) {
                $this->items[] = [
                    'id'      => $entity->getId(),
                    'href'    => $this->controller->getUrl( 'edit', [ 'id' => $entity->getId() ] ),
                    'columns' => $this->getColumns( $entity ),
                ];
            }
        }

        return $this->items;
    }
    ...

-

class ProductListTable extends ListTable {
    protected $tableKeys = array(
        'name',
        'sku',
        'price',
        'brand',
        'status',
        'date_added',
    );
    ...

Then within a controller method I create an instance of the class with:

new ProductListTable( $this );

There's just something I don't like about having to pass in the entire controller instance so I can use some of the methods/services within it. On the other hand, there are quite a few dependencies including some services I'd need to inject through the constructor if I don't inject the controller.

So what I'd like to know is, is it bad practice to inject the controller or is that preferable to injecting 5/6 dependencies as separate function arguments for the constructor? Or does the fact I require these dependencies within the ListTable class mean I'm going about it the wrong way?

Answer 1

controller injection

There are almost none use cases where injecting the controller is a good practice in my opinion.

• controller should be in control, and not to be controlled. By design you should glue all the part inside the controller and give order in a way.
• I assume that most of the functionality you need from the controller, are just another services and not functionality of the controller itself. If it is so then it better to inject the Container object and adding the ContainerAwareInterface
• Still in the end it would be best to inject all 5-6 separate dependencies, because you won't hide your dependencies and your code would be easier to test.

multiple dependencies

If you start to inject many dependencies to an object most of the times it means you are giving to many responsibilities to that object. In general I would say that after 5 dependencies stop and think if it couldn't be grouped in new objects responsible for some parts of the logic.

Lets have an scenario:

• you are working with data that are all in one language, suddenly your client wants to add multilang support.
• you pass all the separate dependencies to our ListTable class, lets say repository, authUser, somethingOther ...
• now you realize that you need to pass the current language, so you need to add dependency (object or string doesnt matter).

Best solution in this case would be to create a new class to handle all the responsibilities of retrieving the correct data to display. Data gathering is one of the things that your ListTable should not do, as its main responsibility should be preparing/digesting the data for easy visualization.

On an example:

Class SecureDataProvider implements LocaleAwareInterface 
{
   public function __construct(ObjectRepository $repository, User $authentificatedUser, $locale) 
   {
    ...
   }
   public function getLatestRecords() 
   {
      // checks if authenticated user got access to this operation or whatever
      // call the repository the get the record sorted by createdAt, passing the locale as query parameter to have the results in requested locale
      // ...

      return ArrayCollection($entities);
   }
}

now you would need to pass only this object to your ListTable.

new ProductListTable(new SecureDataProvider($articlesRepository, ..., ...) );

design of your class

As I said and above you need to separate your responsibilities. The listTable tries to do too much.

On another note visualizing a table is the VIEW in the MVC, and the best place to handle it is in the template.

The purpose of your class should be preparing the data, the way how these data would be shown is the point of View layer (or response).1 The data could in the end be in

• table or list
• graph (headers as labels),
• json, xml or whatever other format from API.

That said the name should be probably changed to reflect the aspect of preparing the data and leave out the part what it will be in the end (table).

If you use twig, you probably would want to make macros for handling the different visual representation

A great reading material on OOP design: http://williamdurand.fr/2013/07/30/from-stupid-to-solid-code/