【问题标题】:SOLID - does Single responsibility principle apply to methods in a class?SOLID - 单一责任原则是否适用于类中的方法?
【发布时间】:2015-06-16 07:58:47
【问题描述】:

我不确定我班级中的这种方法是否违反单一责任原则,

public function save(Note $note)
{
    if (!_id($note->getid())) {

        $note->setid(idGenerate('note'));

        $q = $this->db->insert($this->table)
                      ->field('id', $note->getid(), 'id');

    } else {
        $q = $this->db->update($this->table)
                      ->where('AND', 'id', '=', $note->getid(), 'id');
    }

    $q->field('title', $note->getTitle())
      ->field('content', $note->getContent());

    $this->db->execute($q);

    return $note;
}

基本上它在一个方法中完成两项工作 - 插入或更新。

我应该将其分开分成两种方法来遵守单一责任原则吗?

但 SRP 仅适用于,不是吗?它适用于类中的方法吗?

建议零售价-

一个类应该只有一个职责(即只有一个 软件规范的潜在变化应该能够 影响类的规范)

编辑:

另一种列出笔记(包括许多不同类型的列表)、搜索笔记等的方法......

public function getBy(array $params = array())
{
    $q = $this->db->select($this->table . ' n')
                  ->field('title')
                  ->field('content')
                  ->field('creator', 'creator', 'id')
                  ->field('created_on')
                  ->field('updated_on');

    if (isset($params['id'])) {
        if (!is_array($params['id'])) {
            $params['id'] = array($params['id']);
        }

        $q->where('AND', 'id', 'IN', $params['id'], 'id');
    }

    if (isset($params['user_id'])) {
        if (!is_array($params['user_id'])) {
            $params['user_id'] = array($params['user_id']);
        }

        # Handling of type of list: created / received
        if (isset($params['type']) && $params['type'] == 'received') {
            $q
                ->join(
                    'inner',
                    $this->table_share_link . ' s',
                    's.target_id = n.id AND s.target_type = \'note\''
                )
                ->join(
                    'inner',
                    $this->table_share_link_permission . ' p',
                    'p.share_id = s.share_id'
                )
                # Is it useful to know the permission assigned?
                ->field('p.permission')
                # We don't want get back own created note
                ->where('AND', 'n.creator', 'NOT IN', $params['user_id'], 'uuid');
            ;

            $identity_id = $params['user_id'];

            # Handling of group sharing
            if (isset($params['user_group_id']) /*&& count($params['user_group_id'])*/) {
                if (!is_array($params['user_group_id'])) {
                    $params['user_group_id'] = array($params['user_group_uuid']);
                }

                $identity_id = array_merge($identity_id, $params['user_group_id']);
            }

             $q->where('AND', 'p.identity_id', 'IN', $identity_id, 'id');

        } else {
            $q->where('AND', 'n.creator', 'IN', $params['user_id'], 'id');
        }
    }

    # If string search by title
    if (isset($params['find']) && $params['find']) {
        $q->where('AND', 'n.title', 'LIKE', '%' . $params['find'] . '%');
    }

    # Handling of sorting
    if (isset($params['order'])) {
        if ($params['order'] == 'title') {
            $orderStr = 'n.title';

        } else {
            $orderStr = 'n.updated_on';
        }

        if ($params['order'] == 'title') {
            $orderStr = 'n.title';

        } else {
            $orderStr = 'n.updated_on';
        }

        $q->orderBy($orderStr);

    } else {
        // Default sorting
        $q->orderBy('n.updated_on DESC');
    }

    if (isset($params['limit'])) {
        $q->limit($params['limit'], isset($params['offset']) ? $params['offset'] : 0);
    }

    $res = $this->db->execute($q);

    $notes = array();

    while ($row = $res->fetchRow()) {
        $notes[$row->uuid] = $this->fromRow($row);
    }

    return $notes;
}

【问题讨论】:

    标签: php oop solid-principles single-responsibility-principle


    【解决方案1】:

    方法将笔记保存到数据库。如果这就是它应该做的,那么这是一个单一的责任,实施很好。您需要将决定是否插入或更新的逻辑放在某处,这似乎是一个不错的地方。

    只有当您需要在没有隐式决策逻辑的情况下显式执行插入或更新时,才值得将这两个分离为可以单独调用的不同方法。但是目前,将它们保持在相同的方法中可以简化代码(因为后半部分是共享的),所以这可能是最好的实现。

    示例:

    public function save(Note $note) {
        if (..) {
            $this->insert($note);
        } else {
            $this->update($note);
        }
    }
    
    public function insert(Note $note) {
        ..
    }
    
    public function update(Note $note) {
        ..
    }
    

    如果您有时出于某种原因需要显式调用insertupdate,上述内容将是有意义的。不过,SRP 并不是这种分离的真正原因。

    【讨论】:

    • 谢谢。我上面编辑中的方法怎么样。例如,它用于同时列出笔记和搜索笔记。可以这样做还是应该将它们分开?是否违反了 SRP?
    • 嗯,该方法的职责是获取一组搜索条件并返回一个笔记列表。它总是一样的。即使实现可能相当复杂并且可能被重构为单独的清洁方法,也不会违反 SRP。简而言之,SRP 意味着您应该能够用一句话描述方法/类/模块的作用。一旦你需要用this X does foo, bar, baz and it also make coffee来描述它,它可能违反了SRP。
    • “接受参数 X 并返回结果 Y” 是一项职责。 太多的例子是:“这个类管理数据库连接,序列化数据,渲染模板,缓存响应”
    • 从“持久存储”的角度考虑这一点很有用。您可以创建一个Note 对象在内存中 并随心所欲地使用它,但是一旦您的脚本结束,该对象就会消失。因为它没有在任何地方持久化。数据库(或任何其他存储机制)只是保留该对象,以便您以后可以再次使用它。它只是意味着“保存到数据库”,但从概念上讲,它可以帮助区分 “数据库是我的核心”“数据库仅保存我的对象数据”
    • 就这种思维方式为何有用而言:您的getBy 方法目前专注于构建一个巨大的SQL 查询,该查询将返回您想要的确切数据,因此这种方法非常复杂。但是,如果您根据返回对象数组的方法来考虑它,那么这些对象是如何产生的并不重要。您可能可以将该方法分解为执行几个较小的查询,然后将结果组装成对象,而不是要求 SQL 在一个结果中准确返回您的对象数据。
    【解决方案2】:

    SOLID 原则适用于类级别的术语,它们没有明确说明方法。 SRP 本身指出,类应该有一个改变的理由,所以只要您可以替换包装到一个类中的职责,就可以了。

    考虑一下:

    $userMapper = new Mapper\MySQL();
    // or
    $userMapper = new Mapper\Mongo();
    // or
    $userMapper = new Mapper\ArangoDb();
    
    $userService = new UserService($userMapper);
    

    所有这些映射器都实现一个接口并承担一项职责——它们为用户进行抽象存储访问。因此,映射器有一个改变的理由,因为您可以轻松地交换它们。

    您的案例通常与 SRP 无关。它更多地是关于最佳实践。好吧,关于方法的最佳实践表明,他们应该尽可能只做一件事,并尽可能少地接受参数。这样更容易阅读和查找错误。

    还有一个原则叫做Principle of Least Astonishment。它只是说明方法名称应该明确地执行其名称所暗示的内容。

    回到您的代码示例:

    save() 表示这完全是关于数据保存(更新现有记录),而不是创建。通过在此处进行插入和更新,您会破坏 PoLA。

    就是这样,当您明确调用insert() 时,您知道并期望它会添加一条新记录。 update() 方法也是如此 - 你知道并期望它会更新一个方法,它不会创建一个新方法。

    因此我不会在save() 中同时做这两件事。如果我想更新记录,我会打电话给update()。如果我想创建一条记录,我会打电话给insert()

    【讨论】:

    • 我不同意save 仅暗示插入或更新。 save 对我来说等同于 persist,在这种情况下,我不关心底层实现是做什么的,只要我下次请求时能够再次检索该数据。
    • @deceze 那么你会如何评论save() 关于 PoLS 呢?并且在单独测试时,它是否仍然会比insert()update() 两种方法更好(在期望和可读性方面)?
    • 这个操作有一个很常见的术语:upsert。我不知道是否有人曾经对 upsert 所做的事情感到惊讶。我认为save 的当前实现并不合理(基于对象是否具有 id,而不是基于该 id 是否实际存在于数据库中),但操作本身非常简单且不言自明如果您将数据库视为抽象对象存储。
    猜你喜欢
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    相关资源
    最近更新 更多