できる先輩エンジニアにレビューをいただくことが多い、(僕のような)ビギナーが抜け漏らしがち&&大事なポイントを、チェックリストにまとめました。今一度自分のコードの質を上げるのに役立てたい。
MVC系
- view上のロジックをcontroller or helperに移すことは可能か(controller時点で計算可能なものか)
- controller上のロジックをmodelに移すことは可能か(特定のオブジェクトに関する動作か)
DRY系
- 便利メソッドやイディオムで完結に言い換えられないか(便利メソッド参照: http://qiita.com/jnchito/items/dedb3b889ab226933ccf )(←一度目を通してどういったものがあるのか知るのおすすめします。)
- メソッドの適用範囲(よりpublicに/よりprivateに)
- メソッドの役割の明確さ
- メソッドの汎用性/独立性(前提条件や他のメソッドに依存おらず、汎用的に使えるか。)
- 重複したコードが存在しないか
- call_back使えないか
SQL系
- n+1 Queryないか (allの子たちを全てアクセスしてたりするか。) (参考: http://qiita.com/shunsuke227ono/items/60de21690238aa25e9d4)
- transaction必要か
- インスタンスまるまる召喚する必要ある？(existsとかで済まないか)
命名系
命名の参考: http://qiita.com/jnchito/items/459d58ba652bf4763820
- より正確に変数/methodの意図を特定できないか
- より短く端的に言えないか
DB系
- defaultいらない？
- null:falseいらない？
- 新しいscheme変更に伴う既存データ変更作業必要か(rake task等による)
柔軟性
- 定数→settings.yml?enum?
- 文言→locales
- 想定できる範囲の仕様変更に耐えやすいか
変更に伴うこと
- 影響範囲を列挙し確認したか
- 変更に伴い該当部分のspec(テスト)変更したか
初心者向けのこちらも参照
随時更新/追加していきます。