如何改进大型 CRUD 应用程序中的控制器代码？

Question

我大部分时间都在使用 CRUD 功能的 Web 应用程序上工作。每次新建网站都差不多。

例如我有这段代码，我该如何改进它？

在 CRUD 应用程序中使用这样的代码真的是不好的做法还是正常的？

我使用 Symfony 3.0 和 Doctrine 2.5。

/**
 * @Route("/", name="contacts")
 */
public function listAction(Request $request)
{
    /* @var $em EntityManager */
    $em = $this->getDoctrine()->getManager();
    $searchTerm = $request->get('q');
    $currentGroupId = $request->get('g');

    $qb = $em->createQueryBuilder()
        ->select('c')
        ->from('AppBundle:Contact', 'c')
        ->leftJoin('c.phones', 'p');

    // Get current group from session if not passed by request
    if (empty($currentGroupId)) {
        $request->getSession()->get('contact_current_group_id');
    }

    // Get search term from session if not passed by request
    if (empty($searchTerm)) {
        $request->getSession()->get('contact_search_term');
    }

    // Search if search term is not empty
    if (!empty($searchTerm)) {
        $orX = $qb->expr()->orX();
        $orX->add($qb->expr()->like('c.name', ':searchTerm'));
        $orX->add($qb->expr()->like('p.number', ':searchTerm'));
        $qb->andWhere($orX);
        $qb->setParameter('searchTerm', '%'.$searchTerm.'%');
    }

    // Get and check current group
    $currentGroup = null;
    if ($currentGroupId) {
        $currentGroup = $em->getRepository('AppBundle:ContactGroup')
            ->findOneBy(['enabled' => true, 'id' => $currentGroupId]);

        if ($currentGroup) {
            $qb->andWhere(':groupId MEMBER OF c.groups');
            $qb->setParameter('groupId', $currentGroupId);
        }
    }

    // Get contacts / Pagination
    $paginator = $this->get('knp_paginator');
    $contacts = $paginator->paginate($qb->getQuery(),
        $request->query->getInt('page', 1), 25);

    // Get all groups
    $groups = $em->getRepository('AppBundle:ContactGroup')
        ->findBy(['enabled' => true]);

    return $this->render('contact/list.html.twig', [
        'currentGroup' => $currentGroup,
        'searchTerm' => $searchTerm,
        'contacts' => $contacts,
        'groups' => $groups,
    ]);
}

更新 1

编辑：我对代码做了一些修改。可以做些什么来改进代码呢？您的最佳做法是什么？

/**
 * @Route("/", name="contacts")
 */
public function listAction(Request $request)
{
    /* @var $em EntityManager */
    $em = $this->getDoctrine()->getManager();
    /* @var $repo ContactRepository */
    $repoContact = $em->getRepository('AppBundle:Contact');
    /* @var $repo ContactGroupRepository */
    $repoGroup = $em->getRepository('AppBundle:ContactGroup');

    // Request parameters
    $searchTerm = $request->get('q', $request->getSession()->get('contact_search_term'));;
    $currentGroupId = $request->get('g', $request->getSession()->get('contact_current_group_id'));

    // Queries
    $allGroups = $repoGroup->findByEnabled();
    $currentGroup = $repoGroup->findOneByEnabledAndId($currentGroupId);
    $contactsQuery = $repoContact->createListQuery($searchTerm, $currentGroup);

    // Pagination
    $paginator = $this->get('knp_paginator');
    $contacts = $paginator->paginate($contactsQuery, $request->query->getInt('page', 1), 25);

    return $this->render('contact/list.html.twig', [
        'currentGroup' => $currentGroup,
        'searchTerm' => $searchTerm,
        'contacts' => $contacts,
        'groups' => $allGroups,
    ]);
}

Answer 1

这确实是不好的做法。你永远不应该在 Controller 中构建你的查询。将这些东西移动到一个存储库（在你的例子中是 ContactRepository）并将变量从控制器传递到一个名为 "createSearchQuery" 的函数。在那里，您可以构建查询并将其 return 发送到控制器。从那里您可以将其传递给 paginator.

总的来说 - 尝试将尽可能多的逻辑移出控制器。在尽可能小的功能中。这有助于其他开发人员理解您的代码并使代码更好地进行测试。

Answer 2

这就是所谓的基于意见的问题，可能最终会被关闭。但我从不羞于表达自己的意见。

你的代码其实很普通。 Symfony 是一个基于 Request/Response 的框架，其中控制器负责根据请求生成响应。这正是您的代码所做的。它很容易阅读，并且很清楚它的作用。坦率地说，如果其他开发人员无法查看您的代码并弄清楚发生了什么，那么该开发人员无论如何都不应该搞乱该项目。

您的方法的一个缺点是，如果您最终确实对联系人实体的建模方式进行了重大更改，那么您可能需要搜索和更新大量代码，很容易忽略某些内容。您的方法也可能导致额外的代码重复。例如，如果有其他控制器操作需要启用联系人组，那么您最终将重复相同的查询。如果您对代码感到满意并且项目相当稳定，这又可能没问题。

如果你想写一些命令，你可能也会遇到问题。您最终将不得不从控制器中获取 copy/paste 代码，并再次以重复代码告终。同样，如果您曾经决定做一些事情，比如添加 REST api。

因此，如果您确实想稍微改进一下（正如@tooni 所建议的那样），将功能移至存储库将是一个很好的起点。使用存储库可以让您隔离查询特定的功能，并可能避免重复代码。

在您的情况下，您可以将联系人存储库定义为服务，从而导致：

$contactRepo = $this->get('contact_repository');
$contactQuery = $contactRepo->createQuery($currentGroup,$searchTerm);
$contactGroups = $contactRepo->getEnabledContactGroups();

通过这种方法，您的控制器代码将变得更简单。所有查询构建的东西都被移到了存储库中。您可以共享一些功能。一个更微妙的优势是您的控制器不再需要知道 'AppBundle:Contact'。它只知道它正在获取某种联系人查询。事实上，您的控制器根本不再直接依赖于 Doctrine。

如果你真的想进入它，那么将你的控制器定义为服务并注入存储库和分页器服务。现在你的控制器也独立于依赖注入容器。

总而言之，如果您的代码适合您并且维护它不是一个大问题，那么坚持使用它。如果您想尝试更复杂的方法，请打开存储库，看看会发生什么。

如何改进大型 CRUD 应用程序中的控制器代码？

How to improve controller code in large CRUD applications?

doctrine

coding-style

symfony

doctrine-orm

更新 1