CakePHPでスキニーコントローラと脂肪モデルを作成しようとするガイダンス

Question

私はCakeとMVCの一般的な話に新しくなりました。私は行かなくても良い習慣を確立したい。良い習慣の中にはコントローラをリーンにして、モデルを太くすることがあります。しかし、それは私のようなnoobのための動きのターゲットのビットです。あるモデルから別のモデルに情報を渡す必要がある場合は、すべてをコントローラにダンプするだけですか？それをモデルで動作させるようにしてください。

ここには、私が整理しようとしている混乱の一種の例があります。

すべてはコントローラ内にあるようなと思われますが、間違っている可能性があります。このアクションはメンバのリストを取得し、それをビューに送信します。ビューでは、アカウントを「有効にする」メンバーをチェックすることができます。 ACLなし、単純な認証。私は、 "サブ管理者"はdbフィールド、client_idを使用して管理できるユーザーのみを確認するようにしています。私が使用している2つのモデルはUserとClientです。私の目には

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％ではないと感じることはできません。

Answer 1

私はあなたが1つのクライアントだけを取得したいとは思わない？

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

これはすべてを見つけるはずです。私は多くのユーザーがいる場合に改ページを使用するようにこれを改善しました。私はこれで間違っているかもしれませんが、このコードを見ることであなたの団体も実際の目標も私には分かりません。私は、例えばTicketモデルがどのように他のデータに関連付けられているかわかりません。しかし、私はController :: usesを使っていると思います。

$ aのような恐ろしい変数名を使用しないでください。大したコードブロックやアプリケーションでは、その意味を知ることはできません。また、クライアントデータ$ membersを含む配列に名前を付けました。なぜ、$クライアントではないのですか？これを読むにはClean Code、CakePHPではCakePHP coding standardsに従うことをお勧めします。

目標を記述すると、リファクタリングできる可能性が高いと思います。チケットにアクセスできるようにクライアントをアクティブにしたい場合は、なぜチケットやクライアントコントローラ/モデルで実行していないのですか？

また、この膨大な量のインラインコメントは、それが助けてくれるよりも混乱を招いています。きれいで読みやすいコードを書くと、コードはそれ自体のために話すでしょう。あなたはスーパーコンプレックスコードやスーパーコンプレックスの数学を行っていません。繰り返しますが、私はあなたに "クリーンコード"を読ませてもらうことができます。私の意見では、すべての開発者にとって「読んでおく」必要があります。

<?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を渡すこともできますが、ここでは遅れています。 :)あなたのコードの大まかなリファクタリングとしてこれを取って、私が何を変えたのか、もっと重要な理由について考えてみましょう。

スキンコントローラと脂肪モデルcheck our pluginsが適切に表示されるようにするには、ユーザープラグインのUsersControllerとModelを使用すると、より大きな画像が得られる場合があります。