Upgrade to Pro
— share decks privately, control downloads, hide ads and more …
Speaker Deck
Features
Speaker Deck
PRO
Sign in
Sign up for free
Search
Search
What we learned from code review
Search
Hisashi Kamezawa
March 24, 2018
7
1.8k
What we learned from code review
Hisashi Kamezawa
March 24, 2018
Tweet
Share
More Decks by Hisashi Kamezawa
See All by Hisashi Kamezawa
Assert First
hisas
0
1k
Featured
See All Featured
Navigating Team Friction
lara
183
14k
Save Time (by Creating Custom Rails Generators)
garrettdimon
PRO
27
840
[RailsConf 2023] Rails as a piece of cake
palkan
52
4.9k
Design and Strategy: How to Deal with People Who Don’t "Get" Design
morganepeng
126
18k
Fashionably flexible responsive web design (full day workshop)
malarkey
405
65k
Sharpening the Axe: The Primacy of Toolmaking
bcantrill
38
1.8k
A designer walks into a library…
pauljervisheath
204
24k
実際に使うSQLの書き方 徹底解説 / pgcon21j-tutorial
soudai
169
50k
Cheating the UX When There Is Nothing More to Optimize - PixelPioneers
stephaniewalter
280
13k
Adopting Sorbet at Scale
ufuk
73
9.1k
GraphQLの誤解/rethinking-graphql
sonatard
67
10k
Dealing with People You Can't Stand - Big Design 2015
cassininazir
364
24k
Transcript
永和のコードレビューから学んだ新人が語る Railsアプリケーションで押さえるべきポイント (株)永和システムマネジメント 亀澤 尚志 (@hisas) / 沼田 周(@swamp09) Rails
Developers Meetup 2018: Day 1 2018/03/24(土)
永和のコードレビューから学んだ新人が語る Railsアプリケーションで押さえるべきポイント (株)永和システムマネジメント 亀澤 尚志 (@hisas) / 沼田 周(@swamp09) Rails
Developers Meetup 2018: Day 1 2018/03/24(土)
会社紹介
イベントへの参加補助
None
https://www.wantedly.com/companies/esminc
永和のコードレビューから学んだ新人が語る Railsアプリケーションで押さえるべきポイント (株)永和システムマネジメント 亀澤 尚志 (@hisas) / 沼田 周(@swamp09) Rails
Developers Meetup 2018: Day 1 2018/03/24(土)
自己紹介 名前: 亀澤 尚志 GitHub: hisas Rails歴 1年半 名前: 沼田
周 GitHub: swamp09 Rails歴 1年
永和のコードレビューから学んだ新人が語る Railsアプリケーションで押さえるべきポイント (株)永和システムマネジメント 亀澤 尚志 (@hisas) / 沼田 周(@swamp09) Rails
Developers Meetup 2018: Day 1 2018/03/24(土)
諸注意 - 実際のプロジェクトであったレビューですが、一部改変しています。 - 解釈など間違っている部分があればご指摘ください。 - 永和の1プロジェクトのお話です。
None
目次 - パフォーマンス改善 - リーダブルコード - メンテナンス - エラーハンドリング
パフォーマンス改善
N+1 - eager_loading を使ってクエリ回数を最小限にする - rails server のログをチェックする習慣を付ける
余計なインスタンス生成 - データ有無の確認は余計なインスタンスを生成しない exists? を使う
ループ中のDBアクセス - ループ中で何度もDBにアクセスするとかなり重くなる - Oracle の場合、SQLのIN句に入れられるのは一度に1000件
count, length, size の使い分け - count はSQLを発行して数え上げるのでレコード取得時は length, size を使う
- count はレコード取得は不要で、件数のみの場合に使う
any? と empty? - AR の empty? はレコード取得前ではCOUNT方式のSQLを発 行する -
気になったら Rails の中のコードを読んでみる
遅延評価 - lazy メソッドにより遅延評価して無駄な処理を避ける https://github.com/holiday-jp/holiday_jp-ruby
インデックス - データ量が多く、よく検索が行われる箇所はインデックスを貼 るとよい - インデックスの書き込みに時間がかかるので注意
リーダブルコード
命名規則 - 省略した名前は読み手に負荷がかかるので絶対に使わない - 下手に抽象化するとメソッド名と中身が乖離してしまう
コーディング規約 - メンテナンスする上でスタイルが統一されていることは大切 - PRの本質ではない細かい所で指摘を受けないようにする
条件式 - unless で複数条件は読みにくく頭を使うので避ける
DRY - 二カ所以上で重複しているものを一つにまとめることで変更漏 れを無くす
メンテナンス
データ変更処理をmigrationに書かない - マイグレーションでモデルを使用するのは危険
ロールバック - change だと、rollback できないものもある - up、downと使い分ける必要がある
OSS へのモンキーパッチ - モンキーパッチを当てるより、本家リポジトリにPRを送ってそ のブランチを使用した方が良い - 将来マージされる可能性があり、メンテナンスしやすい - 特定の言語のライブラリに限らない
エラーハンドリング
updateとupdate! - update! は内部でsave!を呼び出している - update! は例外を投げるため。失敗時に気づける
コールバックがスキップされる - コールバックされないメソッドがある - されないことを理解した上で十分気をつけて使用する - updated_at が更新されないので、更新したい場合は自分で 書く必要がある
良いエラーメッセージ - エラーメッセージは、あとで追跡調査をしやすいように
https://www.wantedly.com/companies/esminc