search

UedaKouta / phalcon_work

0 stars 0 forks source link

todo アプリにて　追加したソースコード #8

Open UedaKouta opened 4 years ago

UedaKouta commented 4 years ago

今回、phalconの機能を学習するために、 phalconのサンプルのアプリケーション　INVO　にtodoという機能を追加して動作するようにしました。 項目をDBテーブルに登録・更新・削除を行うものです。

追加・修正したソースコードがわかりやすいようにコメントを入れています。

●参考元のinvoアプリ https://teruchiphalcon-docs.readthedocs.io/ja/latest/reference/tutorial-invo.html

サイト見るだけでは分からなかったので公式のgithubからcloneして、元を動かせるようにしました。 ●github https://github.com/phalcon/invo/tree/0.6.x

mokawa-rosso commented 4 years ago

バリデーションチェックに含まれているとは思うのですが、csrf対策を入れた方が良いかもです。 https://qiita.com/nise_nabe/items/0ce9128c71a8e5da1fd7

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L24

1と2が何なのかが分かりづらいと思いますので、定数とかで書いた方が良いと思います。

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L76

こちらも1が何かが分かりづらいので、定数で書いた方が良いと思います。

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L17-L20

phpdocはパラメータ含めて書いた方が良いと思います。

    /**
     * タスク一覧表示
     * @param string $statusparam パラメータ説明文
     */
mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L106-L107

1番目の$todoへの代入は消し忘れですよね？

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L122

ここに書くよりはmodelのbeforeUpdateに書いた方が良いかもです。 バージョン違いますが、２系でも使えたと思いました。 https://docs.phalcon.io/3.4/ja-jp/db-models-events

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L158-L174

この処理複数回（２回？）出ている気がしますので、privateで切り出しても良いかもです。

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L186-L187

1番目の$todoへの代入は消し忘れですよね？

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/controllers/TodosController.php#L254

細かい所なのですが、下記２点言われる可能性が高いです。 ・文字列はシングルクォーテーションで括ってください。 　（ダブルクォーテーションだと変数展開する関係上わずかに速度が落ちるからとか。。） ・文字列結合のピリオド前後には半角スペースを入れてください。


$this->flash->success('id:' . $id . ' was deleted');
mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/library/Elements.php#L19-L20

他のところは配列の終わり？にカンマつけていましたので、こちらもあわせましょうか。 (現場により、入れないでくれと言われる場合もありますが。。）

                'caption' => 'Home',
                'action' => 'index',
mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/plugins/SecurityPlugin.php#L76-L84

細かい所なのですが、他のところは配列を「[]」で書いていますので、ここもあわせましょうか。

mokawa-rosso commented 4 years ago

https://github.com/UedaKouta/phalcon_work/blob/0c7225433f8c650f2920f60e6500eea75ef5ee47/todo/app/views/todos/index.volt#L11-L13

status == 数字の部分が一瞬分かりづらいかと思いますので、可能でしたら定数を定義してconstantを使ってみませんか？ https://docs.phalcon.io/3.4/ja-jp/volt

mokawa-rosso commented 4 years ago

プルリクエストにないところなのですが、 idにprimary key指定しませんか？

CREATE TABLE `todo` (
  `id` int(10) UNSIGNED NOT NULL,