Paytner Tech Blog

ペイトナーのテックブログです

Railsのコードレビューを受けてみて勉強になったところ

ペイトナーのエンジニアの井齊です。
プロダクトのコード品質を高め維持していくためには、コードレビューが欠かせないですよね。

ペイトナーでは、バックエンドにRailsを使って開発しています。
今回は、コードレビューでコメントをもらった指摘事項をまとめました。

①配列展開メソッドで、 indexの開始の値を指定したい時にeach.with_indexを使う

eachなどを使って配列展開する時に、indexを取得したい場面がありますよね。その時に、each_with_indexを使って対応しようとしていました。
例としては以下のようなケースです。

before

drinks = ["コーラ", "サイダー", "アップルジュース"] 
drinks.each_with_index do |drink, index|
    p "#{index + 1}番目: #{drink}"
end

出力結果はこんな感じでしょうか

1番目: コーラ
2番目: サイダー
3番目: アップルジュース

もちろん、これでもやりたいことは達成できるのですが、今回のように1番目から始めたい場合はindex + 1で指定する必要があります。
そこで、each.with_indexの出番です。

after

drinks = ["コーラ", "サイダー", "アップルジュース"] 
drinks.each.with_index(1) do |drink, index|
    p "#{index}番目:#{drink}"
end

each.with_index(数値)を使うと、indexの最初の値を指定することができます。

② if文の結果を変数に格納する

コントローラー層でmail_flagのパラメータで条件分岐する実装を例として挙げました。
一方はメール送信の関数を実行して「メール送信しました」のメッセージをリダイレクト先で表示させ、
もう片方はメール送信せずに「完了しました」のメッセージをリダイレクト先で表示させる実装です。

before

    if params[:mail_flag]
      send_mail(@user)
      redirect_to complete_path(@user), notice: 'メール送信しました'
    else
      redirect_to complete_path(@user), notice: '完了しました'
    end

beforeも動きますが、if文の結果を変数に格納するとDRYになります。

after

    notice_message = if params[:mail_flag]
      send_mail(@user)
      'メール送信しました'
    else
      '完了しました'
    end
    redirect_to complete_path(@user), notice: notice_message

リダイレクト先は同じで、返すメッセージだけ異なる形だったため、 if文で条件分岐の結果としてメッセージを変数に格納し、そのメッセージをリダイレクト先に渡すことでDRYにできました。

まとめ

今回挙げたようなコードレビューで指摘をもらって、拡張性・変更容易性・視認性など重要ポイントをきっちり見るということが勉強になりました! ここだけでなく、他の実装箇所にも応用できるように頑張っていきたいと思います!

終わりに

ペイトナーは「スモールビジネスにやさしい支払い・請求で新しい挑戦を後押しする」というミッションのもと、
オンライン型ファクタリングサービス「ペイトナー ファクタリング」とクラウド請求書処理お任せサービス「ペイトナー 請求書」を開発・運営しております。

これらのプロダクトを爆速でさらに成長させるために絶賛エンジニア募集中です!
ペイトナーに少しでも興味を持っていただけましたら是非、ペイトナー 採用情報 をご覧ください!!

potent-act-ed6.notion.site