2

参考这条评论，

当一个类有很长的参数列表时，它可能是一种“代码味道”，表明你的类试图做太多事情并且可能没有遵循单一责任原则。如果您的类试图做太多事情，请考虑将您的代码重构为许多相互消耗的较小类。

我应该如何处理下面的这个控制器类 - 它是“试图做太多”吗？

class Controller
{
    public $template;
    public $translation;
    public $auth;
    public $article;
    public $nav;

    public function __construct(Database $connection, $template) 
    {
        $this->template = $template;
        $this->translation = new Translator($connection);
        $this->nav = new Nav($connection);
        $this->article = new Article($connection);
        $this->auth = new Auth($connection);
    }

    public function getHtml() 
    {
        if(isset($_REQUEST['url'])) 
        {
            $item = $this->article->getRow(['url' => 'home','is_admin' => $this->auth->is_admin]);
            include $this->template->path;
        }
    }
}

我怎样才能把它分解成更小的类——如果它是一个控制器，它包含我需要输出页面的这些基本类？

我应该怎么做才能遵循依赖注入的原则？

4

2 回答 2

2

注意：这将是简短版本，因为我在工作。我会在晚上详细说明

所以......您的代码有以下违规行为：

  • SRP（以及扩展 - SoC）：您的控制器负责验证输入、授权、数据收集、用数据填充模板并呈现所述模板。此外，您Article似乎同时负责数据库抽象和域逻辑。

  • LoD：您传递的$connection 唯一 原因是您需要将其传递给其他结构。

  • encapsulation：您的所有类属性都具有公开可见性，并且可以随时更改。

  • 依赖注入：虽然您的“控制器”有几个直接依赖项，但您只是传入模板（实际上不应由适当的 MVC 中的控制器管理）。

  • 全局状态：您的代码依赖于$_REQUEST超全局。

  • 松散耦合：您的代码直接与类的名称和这些类的构造函数的足迹相关联，您在构造函数中进行了初始化。

于 2014-08-01T08:12:36.103 回答
0

TL；DR：我在这里没有看到违反 SRP，但对象组合略有损坏

从我所看到的（这是完整的类列表吗？），$connection没有直接在类中消耗，所以它不应该被注入。

$this->translation而且我看不到$this->nav任何地方的用法。这是复制粘贴的神器吗？

而是注入$connection，我会在这个类之外构造Article和Auth，然后只注入这些，因为你的控制器只直接使用这些，而不是控制器。所以整个班级会是这样的：

class Controller
{
    private $article;
    private $auth;
    private $template;

    public function __construct(Article $article, Auth $auth, $template) 
    {
        $this->article = $article;
        $this->auth = $auth;
        $this->template = $template;
    }

    public function getHtml() 
    {
        if(isset($_REQUEST['url'])) 
        {
            $item = $this->article->getRow(['url' => 'home','is_admin' => $this->auth->is_admin]);
            include $this->template->path;
        }
    }
}

除非您Article是具有 ActiveRecord 模式的域对象，否则我仍然会将其注入$connection并存储在局部变量中。并且仅在您真正需要它时创建新Article对象，即在getHtml方法中。

这样你就不用在构造函数中做太多的工作，只分配局部变量。对象组合在其他地方处理。如果需要，您可以替换Auth.

此外，当您不在构造函数中做太多工作时，您的对象图创建非常便宜。如果您使用某种 DI 容器，当必须同时创建大量对象时，这一点很重要。

于 2014-07-31T21:09:52.540 回答