ブログ

Railsアプリのコードレビューでよく指摘する点

フィヨルドブートキャンプでは最後のカリキュラムで**自分で考えたWebアプリをリリースする**というのがあります。 最後にコードレビューをしているのですが、似た指摘点が多いのでこちらで共有します。 自分でアプリを作ったときにも同じような点が無いか確認すると良いコードになるかもしれません。 ## READMEの情報が足りない railsのwebサービスのリポジトリであれば、下記をちゃんと書こう。 - どんなアプリなのか。 - どんなアプリなのかわかりやすいスクリーンショット。(全ページスクショする必要は無い。) - 動作環境。 - 動かし方。 ## bin/setupで動くようにしよう READMEに細々と動かすためのコマンドが貼ってある場合があるが、基本`bin/setup`を叩けば動くようにしよう。 ## 小文字のApi [RailsでAPIを作る時の設定 - komagataのブログ](https://docs.komagata.org/5430) これ。 ## 名前付け - [クラスの命名のアンチパターン - Qiita](https://qiita.com/magicant/items/8134edf969f9629fa66e) - [プログラミング初心者は変数名やメソッド名を略さない方がいいよ、という話 - give IT a try](https://blog.jnito.com/entry/2020/10/20/092724) ## Fat Controller ControllerはちょっとModelを呼び出すぐらいでややこしい処理はやらない。Modelに処理を移そう。 Controllerにprivateメソッドが増えてきたらFatになってるサイン。1メソッドは10行以内ぐらいを目安に。 ## Fat Model なんでもModelに詰め込んで長くなっていたら注意。 フィヨルドブートキャンプでも**Rubyのオブジェクト指向プログラミング**のカリキュラムでクラス設計を学びますが、それを思い出してモデリングしてみよう。 `app/models/`以下に何も継承しないクラスを作って良いんだよ。 元のコードから単純にメソッドを抜き出して新しいクラスにする人が多いがそのやり方だと大抵変なコードになります。きちんと考えて再設計しましょう。 ## belongs_toしてるものは最初から必須 ```ruby belongs_to :user validates :user, presence: true # 要らない ``` デフォルトでbelongs_toになってるものは必須になってるのでわざわざ必須のvalidatesを書く必要はない。 ## CRUD以外のactionを使っている [DHHはどのようにRailsのコントローラを書くのか | POSTD](https://postd.cc/how-dhh-organizes-his-rails-controllers/) デフォルト以外のactionメソッドがある場合は上記に従ってControllerを分割しよう。 ## 不要なものまでrender :jsonしている ```ruby render json: @user ``` JSON APIを作る時に不要なものまで出してしまうとセキュリティホールになるのでjbuilderで項目を絞ると良い。 ## 不要なActionCableがある ActionCableを使ってない場合は、関連コード丸っと消せるので消すと良い。 ## booleanを返す場合はxxx? booleanを返すメソッドは名前の最後に?をつけると良い。 逆にbooleanを返さないのに`?`が末尾にあると混乱してしまう。 ## 実装のないファイルは消す メソッドのないHelperやTestなど中身のないファイルは丸ごと消すと良い。 ## 不要なroutes ```ruby resources :reports, only: %i(index new create) ``` 使ってないroutesはonlyなどできっちり設定しよう。