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.9k
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
1.1k
Featured
See All Featured
Performance Is Good for Brains [We Love Speed 2024]
tammyeverts
12
1.2k
CSS Pre-Processors: Stylus, Less & Sass
bermonpainter
358
30k
Being A Developer After 40
akosma
91
590k
Six Lessons from altMBA
skipperchong
28
4k
RailsConf 2023
tenderlove
30
1.2k
JavaScript: Past, Present, and Future - NDC Porto 2020
reverentgeek
52
5.6k
Fireside Chat
paigeccino
40
3.7k
Raft: Consensus for Rubyists
vanstee
139
7.1k
Imperfection Machines: The Place of Print at Facebook
scottboms
269
13k
Keith and Marios Guide to Fast Websites
keithpitt
411
22k
Art, The Web, and Tiny UX
lynnandtonic
303
21k
Stop Working from a Prison Cell
hatefulcrawdad
271
21k
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