LoginSignup
3
4

More than 5 years have passed since last update.

CakePHPのbakeが吐くControllerがあんまりな気がする

Last updated at Posted at 2018-09-09

いやいや、bakeのcontrollerなんか使わんよ、という気もしますが、プログラミング初心者には結構ありがたいそうなのです。
しかし、Rails育ちのRESTfulおじさんにはモヤモヤポイントがあって悩んでいます。

環境

CakePHP 3.6.11。
一応、生成のコマンドを載せておきます。
(Productsはチュートリアルからもってきました)

bin/cake bake migration CreateProducts name:string 
description:text created modified
bin/cake migrations migrate
bin/cake bake all products

生成されるコントローラ

<?php
namespace App\Controller;

use App\Controller\AppController;

/**
 * Products Controller
 *
 * @property \App\Model\Table\ProductsTable $Products
 *
 * @method \App\Model\Entity\Product[]|\Cake\Datasource\ResultSetInterface paginate($object = null, array $settings = [])
 */
class ProductsController extends AppController
{

    /**
     * Index method
     *
     * @return \Cake\Http\Response|void
     */
    public function index()
    {
        $products = $this->paginate($this->Products);

        $this->set(compact('products'));
    }

    /**
     * View method
     *
     * @param string|null $id Product id.
     * @return \Cake\Http\Response|void
     * @throws \Cake\Datasource\Exception\RecordNotFoundException When record not found.
     */
    public function view($id = null)
    {
        $product = $this->Products->get($id, [
            'contain' => []
        ]);

        $this->set('product', $product);
    }

    /**
     * Add method
     *
     * @return \Cake\Http\Response|null Redirects on successful add, renders view otherwise.
     */
    public function add()
    {
        $product = $this->Products->newEntity();
        if ($this->request->is('post')) {
            $product = $this->Products->patchEntity($product, $this->request->getData());
            if ($this->Products->save($product)) {
                $this->Flash->success(__('The product has been saved.'));

                return $this->redirect(['action' => 'index']);
            }
            $this->Flash->error(__('The product could not be saved. Please, try again.'));
        }
        $this->set(compact('product'));
    }

    /**
     * Edit method
     *
     * @param string|null $id Product id.
     * @return \Cake\Http\Response|null Redirects on successful edit, renders view otherwise.
     * @throws \Cake\Network\Exception\NotFoundException When record not found.
     */
    public function edit($id = null)
    {
        $product = $this->Products->get($id, [
            'contain' => []
        ]);
        if ($this->request->is(['patch', 'post', 'put'])) {
            $product = $this->Products->patchEntity($product, $this->request->getData());
            if ($this->Products->save($product)) {
                $this->Flash->success(__('The product has been saved.'));

                return $this->redirect(['action' => 'index']);
            }
            $this->Flash->error(__('The product could not be saved. Please, try again.'));
        }
        $this->set(compact('product'));
    }

    /**
     * Delete method
     *
     * @param string|null $id Product id.
     * @return \Cake\Http\Response|null Redirects to index.
     * @throws \Cake\Datasource\Exception\RecordNotFoundException When record not found.
     */
    public function delete($id = null)
    {
        $this->request->allowMethod(['post', 'delete']);
        $product = $this->Products->get($id);
        if ($this->Products->delete($product)) {
            $this->Flash->success(__('The product has been deleted.'));
        } else {
            $this->Flash->error(__('The product could not be deleted. Please, try again.'));
        }

        return $this->redirect(['action' => 'index']);
    }
}

以下のエンドポイントができるのを想定しているものと思います。
「思います」と書いたのはCakePHPでは config/routes.php に書かなくてもメソッドが外から呼べるため、正確にはわからないからです。

  • get /products で一覧を取得
  • get /products/:id で一覧を詳細取得
  • get /products/add で新規追加画面表示
  • post /products/add で新規追加処理
  • get /products/edit で更新画面表示
  • patch or post or put /products/edit で更新処理
  • post or delete /products/:id で削除処理

モヤモヤポイント

ざっと眺めて、嫌な臭いを感じて「よし! 本腰をいれてくぞ!」って気持ちになるのは僕だけではないはず…?
例としてaddをコードレビューしてみる。

public function add()
{
    $product = $this->Products->newEntity();
    # [must] 新規追加画面の表示と新規追加の処理を分けたほうが見通しがよいです
    if ($this->request->is('post')) {
        # [must] routesでpostだけを許可すればこの分岐はいりませんね
        $product = $this->Products->patchEntity($product, $this->request->getData());
        if ($this->Products->save($product)) {
            $this->Flash->success(__('The product has been saved.'));

            return $this->redirect(['action' => 'index']);
        }
        $this->Flash->error(__('The product could not be saved. Please, try again.'));
        # [imo] 成功の場合はindexに、そうでない場合はこのまま、ということですね。
        # 処理が読みづらく感じました。
    }
    # [must] post以外は新規追加画面表示にするというのは乱暴すぎやしませんか?
    # 他は`allowMethod`で弾いてるのにここはなんでやらないんですか?
    $this->set(compact('product'));
}

fatなコントローラはどこでもよく目にしますが、これがそのまま育っていくと思うと、頭が痛くなってきます。
(もちろん実際見ました)
そして経験の浅い方は「CakePHPが出力したコードだ。これでよいのだ!」と思いますよね。

どうすればいいか考える

CakePHPにはRESTfulなルーティングを生成する機能があります。

config/routes.php

$routes->resources('Products');

生成されるエンドポイント。

bin/cake routes | grep product
| products:index                      | /products                               | {"_method":"GET","action":"index","controller":"Products","plugin":null}          |
| products:add                        | /products                               | {"_method":"POST","action":"add","controller":"Products","plugin":null}           |
| products:view                       | /products/:id                           | {"_method":"GET","action":"view","controller":"Products","plugin":null}           |
| products:edit                       | /products/:id                           | {"_method":["PUT","PATCH"],"action":"edit","controller":"Products","plugin":null} |
| products:delete                     | /products/:id                           | {"_method":"DELETE","action":"delete","controller":"Products","plugin":null}      |

そうそう、こういうのでいいんだよ。
これを使えば、if ($this->request->is('post')) {allowMethod を一掃できそうですね。

しかし…

以下のエンドポイントがありません。APIでの利用を想定している、ということでしょう。
(APIの場合、パラメータを入力するための画面はいらないので)

  • get /products/add で新規追加画面表示
  • get /products/edit で更新画面表示

こう…?

$routes->resources('Products', ['map' => [
    'add' => [
        'action' => 'new',
        'method' => 'GET'
    ],
    ':id/edit' => [
        'action' => 'viewEdit',
        'method' => 'GET'
    ]
]]);

edit はもう使われているので、名前が悩ましい…。

| products:index                      | /products                               | {"_method":"GET","action":"index","controller":"Products","plugin":null}          |
| products:add                        | /products                               | {"_method":"POST","action":"add","controller":"Products","plugin":null}           |
| products:view                       | /products/:id                           | {"_method":"GET","action":"view","controller":"Products","plugin":null}           |
| products:edit                       | /products/:id                           | {"_method":["PUT","PATCH"],"action":"edit","controller":"Products","plugin":null} |
| products:delete                     | /products/:id                           | {"_method":"DELETE","action":"delete","controller":"Products","plugin":null}      |
| products:new                        | /products/add                           | {"_method":"GET","action":"new","controller":"Products","plugin":null}            |
| products:viewedit                   | /products/:id/edit                      | {"_method":"GET","action":"viewEdit","controller":"Products","plugin":null}       |

うーん、冗長感あるし、メソッド名がフレームワークで規定されないのが悶々とする。
ベターな解決策があれば教えていただけると幸いです :bow:

参考

Railsの場合。

               sites GET    /sites(.:format)                sites#index
                     POST   /sites(.:format)                sites#create
            new_site GET    /sites/new(.:format)            sites#new
           edit_site GET    /sites/:id/edit(.:format)       sites#edit
                site GET    /sites/:id(.:format)            sites#show
                     PATCH  /sites/:id(.:format)            sites#update
                     PUT    /sites/:id(.:format)            sites#update
                     DELETE /sites/:id(.:format)            sites#destroy

そうそう、こういうのでいいんだよ。

3
4
1

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
3
4