1

我是 Cake 的新手,一般来说也是 MVC 的新手。我想从一开始就养成良好的习惯。良好的习惯包括让控制器保持苗条,并使模型变胖。但对于像我这样的菜鸟来说,这有点移动目标。如果我需要将信息从一个模型传递到另一个模型,我是否只需将所有这些信息都转储到控制器中?试着让它在模型中工作?

这是一个动作,它是我试图解决的那种混乱的一个典型例子。

一切似乎都应该在控制器中,但我可能错了。此操作获取成员列表,将其发送到视图。在视图中,我可以勾选我想要“激活”其帐户的成员。没有 ACL,只是简单的身份验证。我确保“子管理员”只能通过使用 db 字段 client_id 看到他们被允许管理的用户。我使用的两个模型是用户和客户端。

public function activate() {
    if ($this->request->is('get')) {
        $id = $this->Auth->user('id');
        $this->User->id = $id; //make sure current User is the logged in user
        $currentClient = $this->User->field('client_id'); // get client_id based on logged in user
        $members = $this->User->Client->find('first', array( // find users that have the same client_id
            'conditions' => array('id' => $currentClient),
            'recursive' => 1
        ));
        $this->set('clients', $members); // send the users to the view                  
    } else if ($this->request->is('post') || $this->request->is('put')) {
        $members = $this->request->data['Members']; // grab players submitted from push form
        $memberIds = array(); // this will hold the selected users
        foreach($members as $a){
            $memberIds[$a['id']] = $a['id']; // loop over user's that were selected
        }
        $usersToActivate = $this->User->find('all', array( //find user records, based on the array of id's
            'conditions' => array(
                "User.id" => $memberIds
            )
        ));
        $this->Ticket->bulkActivate($usersToActivate); // send array of members into model for processing
        $this->Session->setFlash('Activations sent.', 'default', array('class' => 'success'));
        $this->redirect(array('action' => 'index'));
    }
}

在我看来,它看起来并没有大错特错......而且我已经在模型中进行了一些处理(如实际获取用户记录并生成激活票证的 bulkActivate 所示)。

但我不禁觉得它还不是 100%。

4

1 回答 1

2

我不认为您只想获得一个客户?

$members = $this->User->Client->find('first', array

我想这应该找到所有。在有很多用户的情况下,我已经改进了它以使用分页。我可能对此有误,但通过查看这段代码,我并不清楚您的关联和真正的目标。我不知道例如 Ticket 模型是如何与任何其他数据相关联的。但我猜你正在使用 Controller::uses,你不应该这样做,而是通过它们的关联访问相关模型。

不要使用像 $a 这样可怕的变量名,这很糟糕,没有人会知道这在更大的代码块或应用程序中意味着什么。您还命名了一个包含客户数据 $members 的数组,为什么不命名为 $clients?阅读这篇文章:清洁代码和 CakePHP 我建议您遵循CakePHP 编码标准

描述目标,我认为这可以更好地重构。如果您想激活客户端以访问票证(这就是它的样子),为什么您没有在票证或客户端控制器/模型中完成它?

此外,大量的内联注释只会造成更多的混乱而不是帮助。编写干净易读的代码,代码会自己说话。您还没有在那里完成任何超级复杂的代码或超级复杂的数学运算。再一次,我可以推荐你阅读“清洁代码”,我认为这是每个开发人员的“必读”。

<?php
    // UsersController.php
    public function activate() {
        if ($this->request->is('post') || $this->request->is('put')) {
            $this->User->activate($this->request->data);
            $this->Session->setFlash('Activations sent.', 'default', array('class' => 'success'));
            $this->redirect(array('action' => 'index'));
        }

        this->Paginator->settings['Client'] = array(
            'conditions' => array('id' => $this->Auth->('current_id')),
            'contain' => array(
                'OnlyModelsYouNeedHere'));
        $this->set('clients', $this->Paginator->paginate($this->User->Client)); 
    }
?>

<?php
    // User.php - the model
    public function activate($data) {
        $memberIds = array();
        foreach($data['Members']members as $member) {
            $memberIds[$member['id']] = $member['id'];
        }
        $usersToActivate = $this->find('all', array(
            'conditions' => array(
                'User.id' => $memberIds)));
        return $this->Ticket->bulkActivate($usersToActivate);
    }
?>

传递 id 也可以减少更多,但是,嘿,现在已经晚了。:) 把它当作你代码的粗略重构,想想我改变了什么,更重要的是为什么。

如果您想查看正确的瘦控制器和胖模型示例,请查看我们的插件,用户插件 UsersController 和 Model 可能会给您一个更大的图景。

于 2012-04-27T00:49:49.690 回答