Symfony : should logic about formatting a parameter be in repository or controller?

Question

What is the best practice according the Symfony way ?

Option 1 :

class MyController extends Controller
{
    public function myAction(...)
    {
        // ..
        $example = $this->getDoctrine()->getRepository('AppBundle:Contract')->getContracts(array(
            'company_id'                => $company->getId(),
            'contract_month_date_start'  => date('Y-m-d', strtotime('first day of this month', $contractDate->getTimestamp())),
            'contract_month_date_end'    => date('Y-m-d', strtotime('last day of this month', $contractDate->getTimestamp())),
        ));
        // ..
    }
}

class ExampleRepository extends EntityRepository
{
    public function getContracts($options)
    {

        //..

        $qb->select('contract')
            ->from('AppBundle:Contract', 'contract')
            ->where('contract.companyId = :company_id')
            ->andWhere('contract.startDate < :contract_month_date_end')
            ->andWhere('contract.endDate < :contract_month_date_end')
            ->setParameter('company_id', $options['company_id'])
            ->setParameter('contract_month_date_start', $options['contract_month_date_start'])
            ->setParameter('contract_month_date_end', $options['contract_month_date_end']);

        //..
    }
}

Option 2 :

class MyController extends Controller
{
    public function myAction(...)
    {
        // ..
        $example = $this->getDoctrine()->getRepository('AppBundle:Contract')->getContracts(array(
            'company'      => $company,
            'contractDate' => $contractDate
        ));
        // ..
    }
}


class ExampleRepository extends EntityRepository
{
    public function getContracts($options)
    {

        //..

        $company_id                = $options['company']->getId();
        $contract_month_date_start = date('Y-m-d', strtotime('first day of this month', $options['contractDate']));
        $contract_month_date_end   = date('Y-m-d', strtotime('last day of this month', $options['contractDate']));

        $qb->select('contract')
            ->from('AppBundle:Contract', 'contract')
            ->where('contract.companyId = :company_id')
            ->andWhere('contract.startDate < :contract_month_date_end')
            ->andWhere('contract.endDate < :contract_month_date_end')
            ->setParameter('company_id', $company_id)
            ->setParameter('contract_month_date_start', $contract_month_date_start)
            ->setParameter('contract_month_date_end', $contract_month_date_end);

        //..
    }
}

I feel like the second one is better (less duplicate code between the differents controllers using the repository). But I also feel like it gives too much responsibility to the Repository.

lordrhodos · Answer 1 · 2017-06-26T18:04:33.183

Symfony best practices are not clear regarding your task, but IMHO the following statement could be mapped to your second option

Overall, this means you should aggressively decouple your business logic from the framework while, at the same time, aggressively coupling your controllers and routing to the framework in order to get the most out of it.

As you have already stated, the second option keeps the controllers thinner and moves the logic to the repository level, which is in terms of a Symfony way a totally valid approach.

This best practice statement adds some additional support for option 2

In Symfony applications, business logic is all the custom code you write for your app that's not specific to the framework (e.g. routing and controllers). Domain classes, Doctrine entities and regular PHP classes that are used as services are good examples of business logic.

If you need this formatting at different places around your application another approach could be to create a service class which you can inject into the Repositories where you need it.

Aziule · Accepted Answer · 2017-06-27T08:16:40.980

TL;DR: in your example, your formatting logic should ideally be handled in an intermediate layer.

Here are a few improvements:

First, do not get the repository from your controller. Instead, declare it as a service (for example, in your app/config/services.yml).

Also, you could abstract the access to your data (= your repository) using a new layer. The communication would be done like this:

Controller <=> Business layer <=> Data access.

Controller: responsible for calling the correct methods of the business layer and handling responses (rendering HTML, returning JSON, etc.).
Business layer: responsible for the logic of your app.
Data access: responsible for querying your DB (for example) and getting the data without including any business logic. Can only be accessed by the business layer.

Using such an architecture will help you having a more scalable application without taking too long to implement it.

In your code, it would look like the following:

Your controller, responsible for "routing" the user's request to the correct service

class MyController extends Controller
{
    public function myAction()
    {
        // Accessing the contract manager, previously declared as a service in services.yml
        $contractManager = $this->get('manager.contract');
        $contracts = $contractManager->getMonthlyContracts($company);
    }
}

Your business layer, responsible for executing the logic:

// This class must be declared as a service in services.yml
class ContractManager {
    private $contractRepository;

    public function __construct(ContractRepository $contractRepository)
    {
        $this->contractRepository = $contractRepository;
    }

    public function getMonthlyContracts(Company $company)
    {
        // The business layer is responsible for the logic of fetching the data. Therefore, its methods are business-related (here "find the monthly contracts for company X", which is very specific).
        // It is its role to set the start and end dates, not the controller's
        $contracts = $this->contractRepository->findByCompany(
            $company,
            new \DateTime('first day of this month'),
            new \DateTime('last day of this month')
        );

        // Do some logic with the contracts if required...

        return $contracts;
    }
}

Your repository, responsible for accessing the data:

class ContractRepository extends EntityRepository
{
    // The repository handles basic access to data without any logic, hence the generic "findByCompany" method name
    public function findByCompany(Company $company, \DateTime $from, \DateTime $to)
    {
        $qb->select('contract')
            ->from('AppBundle:Contract', 'contract')
            ->where('contract.company = :company')
            ->andWhere('contract.startDate > :from')
            ->andWhere('contract.endDate < :to')
            ->setParameter('company', $company)
            ->setParameter('from', $from)
            ->setParameter('to', $to);

        //...
    }
}

Please have a look at the Symfony doc about how to declare a repository as a service in your services.yml.

score 0 · Answer 3 · answered Jun 26 '17 at 16:00

0

I prefer to do this kind of job in the presentation layer, if you are using TWIG you should take a look at the date format filter or if it is an api you can work to the presentation layer with JMS serialization as example.

In this manner the business logic don't change and is not affected about change of the presentation logic

My two cents.

answered Jun 26 '17 at 16:00

Matteo

37,680
11
100
115

Maybe I oversimplified my Controller but $company and $contractDate are created just before the call to the repository. They are not data sent by the user. I don't understand how I can use twig for this case... ? – lepix Jun 26 '17 at 16:25
1

hi @lepix sorry for the confusion, the database column is of a string format? so in this case should be the repository to do the work (in the same manner the view should present the logic and adapt the object to something other) – Matteo Jun 26 '17 at 19:18

Symfony : should logic about formatting a parameter be in repository or controller?

3 Answers3