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
5 minutes of I Can Smell Your CMS
philhawksworth
202
19k
What's in a price? How to price your products and services
michaelherold
243
12k
Put a Button on it: Removing Barriers to Going Fast.
kastner
59
3.5k
Become a Pro
speakerdeck
PRO
25
5k
JavaScript: Past, Present, and Future - NDC Porto 2020
reverentgeek
47
5k
Fontdeck: Realign not Redesign
paulrobertlloyd
82
5.2k
"I'm Feeling Lucky" - Building Great Search Experiences for Today's Users (#IAC19)
danielanewman
226
22k
The World Runs on Bad Software
bkeepers
PRO
65
11k
Understanding Cognitive Biases in Performance Measurement
bluesmoon
26
1.4k
The Invisible Side of Design
smashingmag
297
50k
VelocityConf: Rendering Performance Case Studies
addyosmani
325
24k
XXLCSS - How to scale CSS and keep your sanity
sugarenia
246
1.3M
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