シリコンバレー流コードレビューでボコられた話
これがシリコンバレー流コードレビューである――。
界隈では知らない人はいないでしょう、約2年前にシリコンバレーエンジニアの酒井潤さんがコードレビューするという企画をTwitterで募集していたので応募してみました。
ちょうど大学院の課題用に作成したアプリがあったのでDMで依頼したところ、1週間ほどでYouTubeにレビュー動画がアップされました。
「自分のコードがどのように評価されるのか?」
「プロフェッショナルの目線とはどんなものか?」
「そもそもどうやってコードレビューすれば良いものか?」
そんな問いに直面した方は、ぜひこのレビュー内容を参考にしてみてください。
レビュー対象のリポジトリ
レビュー動画
2本合計で約2時間弱の長尺動画ですが、指摘内容は非常に詳細です。
この動画の指摘点を意識することで、レビューを受ける側としてもレビューする側としても大きな成長を得られるでしょう。
アプリの概要
一言で言うとSUUMOをスクレイピングして機械学習で物件の条件と家賃が適正かどうかを判断するアプリです。
今回レビュー依頼したのは、大学院の課題として提出したアプリで、Boston house price
というデータセットを真似したものです。
プロット例(凡例がズレてますがお気になさらず)
--- 以下、指摘内容 ---
コードレビューに入る前に確認すべきポイント
-
README
- ヘッダー項目名の修正:
-
FarFromStation
→minutesOnFootFromStation
-
- ヘッダー項目名の修正:
-
リポジトリ名の命名規則
- ハイフンを避ける:
-
tar.gz
で圧縮時にエラーが発生する可能性がある。 - CI/CDツールで読み込み時に問題が生じることがある。
-
- ハイフンを避ける:
-
.gitignoreファイル
- 不要なファイルを除外:
.DS_Store
.idea
.pyc
- 不要なファイルを除外:
-
フォルダ構成
- スクレイピングと機械学習でフォルダを分けているので、
config
フォルダを共有する。
- スクレイピングと機械学習でフォルダを分けているので、
レビュー指摘内容
動画1
-
インポートの明確化
-
import *
は使用ライブラリが分からないため避ける。
-
-
ライブラリの命名規則
- 独自ライブラリの名称変更例:
-
logger
→ai_logger
※新規参画者が把握しやすくなる。
-
- 独自ライブラリの名称変更例:
-
引数設計
- 固定値の引数を省略:
- 例:
logger
の各レベルにtrace
を渡す処理は、関数内で完結させる。
- 例:
- 固定値の引数を省略:
-
ファイル操作のベストプラクティス
-
パス管理: 文字列連結ではなく
os.path.join()
を使用する。 -
ファイル操作:
with open()
でクローズ処理を保証。 -
エラーハンドリング:
-
base64encode
処理時、text
やnull
が来た場合の対策。
-
-
パス管理: 文字列連結ではなく
-
比較記述のルール
- 大小比較は
小 < 大
の順に記載する。
- 大小比較は
-
コードの簡略化
- 不要なスペースや冗長なコードは削除し、処理の意図を明確にする。
-
ファイル名の一貫性
- ファイル名や変数名に一貫性を持たせ、意図を明確にする。
-
コメント・コード構造
- コードのセクションを明確に分ける(例: 「ファイル名生成」「リスト操作」「削除処理」など)。
- コメントを追加して、各セクションの意図を明確にする。
動画2
-
関数・クラス設計
- クラスにまとめて設計することで、コードの可読性と再利用性を向上。
- 例:
HTMLPage
クラスにget_soap
,generate_csv_data
,backup_csv
などのメソッドを統合。
- 例:
- クラスにまとめて設計することで、コードの可読性と再利用性を向上。
-
ネーミングの改善
- 関数名や変数名を処理内容を反映したものに変更。
-
ハードコーディングの改善
- ハードコーディングされた値を変数や設定ファイルに移動。
-
正規表現の利用
-
re
モジュールを利用して、文字列操作を効率化。
-
-
エラーハンドリング
-
try-except
の範囲を狭めることで、エラーの特定を容易にする。
-
-
リスト操作
- インデックスを直接操作せず、
zip
やリスト内包表記を利用。
- インデックスを直接操作せず、
-
ログ・プリントの改善
-
print
の代わりにsys.stdout.write
を使用。 - カラー出力を分離し、設定ファイルで管理。
-
-
スリープ処理の明確化
-
time.sleep(1)
の1
を変数に置き換え、単位を明示。
-
-
スタイルガイド遵守
- スネークケースの使用を徹底(例:
CSV_FILE_PATH
→csv_file_path
)。
- スネークケースの使用を徹底(例:
酒井潤のレビュー手順
コードレビューを行う際の参考テンプレート:
-
READMEの確認
アプリの概要や目的を把握。 -
リポジトリ全体の構成確認
.gitignore
やフォルダ構成の論理性を確認。 -
主要ファイルの確認
app.py
などのエントリーポイントのコード構造を確認。 -
主要処理の確認
スクレイピングやビジネスロジックが適切に記述されているか確認。 -
スタイルガイドの確認
PEP 8に従っているかをチェック。 -
レビュー指摘内容に戻る
終わりに
プロフェッショナルのレビューを受けることで得られる学びは計り知れません。このレビュー内容を活かして、コード品質をさらに高めていきましょう。
そして本YouTubeのコメント欄を見る限り、特に私のコードを批判するコメントは見当たりませんでした。むしろ有益だという声が多く、平和な世界が広がっています。
「勉強になった」というコメントも多かったので、第三者として他者のコードレビューを見るというもまた一つ有りなんではないでしょうか。
P.S.
指摘内容の修正は行なっていません。どなたでも修正PR出してください。
Discussion