プロのRails開発者に学ぶ！コードレビュー集

　私は現在Happiness Chainというプログラミングスクールに在籍しており、Rails課題でECサイト、Twitterクローンを実装しました。その中でメンターさんから指摘された事をまとめました。間違いがありましたら、ご指摘いただけると嬉しいです。

## 🙆RESTfulな設計にしましょう
　これはitemsコントローラのindexアクションに割り当てられています。beforeとafterどちらとも生成されるpathは`items_path`です。
##### 修正前
```models/routes.rb
  get "/items", to:"items#index"
```

　リソースベースのルーティング`resources`を使う事でCRUDアクションのルーティングを一つ一つ記述しなくても、HTTPメソッドの(`GET`、`POST`、`PUT`、`PATCH`、`DELETE`)がそれぞれ、コントローラの(`index`、`show`、`new`、`edit`、`create`、`update`、`destroy`)というアクションに対応して自動的に生成されます。これによりRESTfulな原則に基づいて設計できます。
```sample.rb
 resources, :items

#ターミナル
$rails routes

    items GET /items(.:format)          items#index 
          POST /items(.:format)         items#create 
 new_item GET /items/new(.:format)      items#new 
edit_item GET /items/:id/edit(.:format) items#edit 
     item GET /items/:id(.:format)      items#show 
          PATCH /items/:id(.:format)    items#update 
          PUT /items/:id(.:format)      items#update 
          DELETE /items/:id(.:format)   items#destroy

```
　特定のアクションだけのルーティングを生成したい場合、`:only` オプションを使用して指定することもできます
##### 修正後
```models/routes.rb
resources, :items, only: %i[index]　 
```

## 🙆アソシエーションを使いましょう
　deviseなら`current_user`でRailsアプリケーションで現在ログインしているユーザーを取得できる。全てのツイートから自分のツイートを取得しようとしている。
##### 修正前
```controllers.sample.ruby
 @my_tweets = Tweet.where(user_id: current_user.id)
```
　Userとツイートは以下のように関連づけているので
```app/models/user.rb
 class User < ApplicationRecord
  has_many :tweets, dependent: :destroy
 end
```
```app/models/tweet.rb
class Tweet < ApplicationRecord
  belongs_to :user
end
```
　current_user.tweetsのようにアソシエーションを使って自身の全てのツイートを取得できます。
##### 修正後
```ontrollers.sample.ruby
 @my_tweets = current_user.tweets
```
https://railsguides.jp/association_basics.html

## 🙆DB制約は厳重にしましょう
 可能な限り`null false`をつけます。
```ruby
  def change
    create_table :users do |t|
      t.string :name, null: false
      t.string :email, null: false
```
　`:user, :email` 属性が存在し（nil や空文字列でない）、データが保存される際に必須であることを指定しています。
```app/models/user.rb
class User < ApplicationRecord
  validates :user, presence: true
  validates :email, presence: true
end
```
ただ、このバリデーションは単に値が存在するかどうかのみを確認しているので、例えば「12ekpddke」のような実際にはemailとして不適切な値でも保存されてしまいます。そのため、正規表現を用いて期待するデータ形式をより厳密に指定します。他にも電話番号、郵便番号、WebサイトのURL、テキストの長さといった値なども厳密に指定する必要があります。生年月日の欄で未来の日付だったり、西暦100年などありえない数値のときもエラーとなるように制約ができます。
```app/models/user.rb
class User < ApplicationRecord
  validates :user, presence: true
  validates :email, presence: true, format: { with: /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i/ }
end
```
https://railsguides.jp/active_record_validations.html

## 🙆空文字で送信されたときはValidationエラーを表示するといいでしょう
```tweets_controller.rb
 def create
    @tweet = current_user.tweets.build(tweet_params)
    if @tweet.save
      redirect_to root_path
      flash[:success] = 'ツイートしました'
    else
   #ステータスコードをバリデーションエラーの場合に返すシンボル表現にする
      render 'index', status: :unprocessable_entity
    end
  end
```
```models/tweet.rb
class Tweet < ApplicationRecord
  validates :text, presence: true
end
```
```tweets/_form.html.slim
  = form_with model: tweet do |f|
/バリデーションエラーメッセージは他のフォームでも使うので共通化するといい
    = render 'layouts/error_messages', model: tweet
/以下省略
```
🙆エラーメッセージに[style](https://getbootstrap.jp/docs/5.0/components/alerts/)を当てると尚いいでしょう。私はエラーメッセージは赤色にしました。
```layouts/_error_messages.html.slim
- if model.errors.any?
  div class="alert alert-danger"
    ul
      - model.errors.full_messages.each do |message|
        li = message
```
　Validationエラーメッセージは日本語に変更できます。
![スクリーンショット 2023-08-22 14.07.04.png](https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/3012370/b35ab4da-6b2d-3caf-7946-7c9d25df788c.png)











## 意識するようになったこと
　Rails scaffoldの`index` `show` `edit` `create` `update` `destroy` がシンプルな構成となっているのが理想であることを知りました。ですのでコントローラはあれやこれや情報を入れないで限りなくシンプルになるように意識しました。また複雑なロジックは全てモデルへ移行するようになりました。
## 開発する上で参考にしたサイト
https://qiita.com/jnchito/items/2b57316b39d92ca0e8dc

https://qiita.com/jnchito/items/dedb3b889ab226933ccf

https://qiita.com/ihatov08/items/b8f60d03bb1ae11b438a

https://railsguides.jp/getting_started.html