Pythonセルフレビューチェックリスト

はじめに

チーム内の需要に応えてセルフレビューを行う際のチェック項目をざっと書き出してみたのですが
外部に公開した方が楽にブラッシュアップできるなと思い投稿しました。なお

• スタイルについてはフォーマッターで強制するので触れません
• 項目は全てyes or noで答えられる形になっています
• 厳しすぎるくらいに書いて「出来るかぎりで 構わないから」という運用です
• エラーハンドリングについての項目は諸事情により省略しています

という前提。

一般

• 文字列のフォーマットをformat関数ではなくf-stringで行っている

• 関数のデフォルト引数にミュータブルオブジェクトを設定していない

• ミュータブルオブジェクトの破壊的メソッドを使用していない

  hoge = [1, 0, 2]
  hoge.sort()  # ×
  fuga = sorted(hoge)  # ○
  

• 比較の式は基準値を右辺に置いている

  if x > 10
  

• 三項演算子/セイウチ演算子/内包表記を濫用していない

  • 通常のif/for文で記載した方法と比較した
  • 三項演算子を or で置換できるか確認した

• listを濫用していない

  • 要素数が不変のものにはtupleを用いている
  • イテレータが欲しい場合はリスト内包表記ではなくジェネレータ式を用いている

• while Trueを使用していない

• パスはstr型ではなくpathlib.Path型で扱っている

• 正規表現におけるグルーピングには名前を使用している

  p = re.compile(r'(?P<word>\b\w+\b)')
  m = p.search( '(((( Lots of punctuation )))' )
  m.group('word')
  

設計

クラス

• SOLIDの原則に従っている
  • 単一責任の原則に従っている
  • 解放/閉鎖の原則に従っている
  • リスコフの置換原則に従っている
  • インタフェース分離の原則に従っている
  • 依存性逆転の原則に従っている

関数(メソッド)

• 単一の機能のみを持っている
• 副作用がない
• 30行以内に収まっている
• 3段以上のネストが存在しない
• 変数は使用する直前に宣言されている
• 変数が使いまわされていない
• 類似性の低い変数同士を1行で宣言していない
• マジックナンバーを使用していない

命名

クラス

• Docstringと整合性が取れている
• 名詞で終わっている
• 名前からクラスの責任を読み取ることが出来る
• 抽象クラスは抽象的な名前、具象クラスは具体的な名前になっている

関数(メソッド)

• Docstringと整合性が取れている
• (bool型以外を返す場合) 動詞から始まっている
• (bool型を返す場合) be動詞/助動詞から始まっている
• 名前から関数の機能もしくは目的を読み取ることが出来る
• 第一引数を指す目的語は省略している
• 外部から呼ばれることのないメソッドには先頭に _ が付いている
• 特殊メソッド以外で __ は使用していない

変数(定数)

• (bool型以外の場合) 名詞で終わっている

• (bool型の場合) be動詞/助動詞から始まっている

  is_empty = False
  can_read = True
  has_changed = True
  

• (イテラブルオブジェクトの場合) 名詞が複数形になっている

• 名前から変数に紐づくオブジェクトを具体的かつ正確に読み取ることが出来る

• 特定の場合に決まった名前を使用している

  • _: 使用しないオブジェクト
  • r, w: リーダー, ライター
  • i, j, k: ループ回数のカウンター
  • c: ループ以外のカウンター
  • x, y, z: 座標
  • cls: クラスメソッドの第一引数
  • self: インスタンスメソッドの第一引数
  • other: selfと同じクラスの別インスタンス

共通

• 予約語/組み込み関数と被っていない

  # クラス変数、インスタンス変数およびメソッドを除く
  

• 英文法に間違いがない

• ofに掛かる言葉は2語以上である

  game_of_thrones = hoge  # ×
  thrones_game = hoge  # ○
  start_of_something_new = hoge  # ○
  

• 略記を使用している場合はその略記が一般的であるか確認した

Docstring

共通

• 名前と整合性が取れている
• そのクラスや関数が知りえない情報を含んでいない
• 文中に含まれる用語の定義を確認した

クラス

• クラスの持つ責任の概要を説明している
• クラスを作成した目的について説明している
• 具体的な処理についてはメソッドのDocstringに委ねている

関数(メソッド)

• 関数の機能もしくは目的の概要を説明している
• 何を受け取って何を返すか説明している

型アノテーション

• 全てのクラス変数、インスタンス変数に型アノテーションが付与されている

• 全ての関数に戻り値の型アノテーションが付与されている

  # 戻り値を指定していない場合は `None`
  # 戻り値を持たない場合は `NoReturn`
  # 一部の特殊メソッドを除く
  

• ジェネリックなクラスについては内部の型も記述している

  hoge: list = ['a', 'b', 'c']  # ×
  hoge: List[str] = ['a', 'b', 'c']  # ○
  fuga: Tuple[int, ...] = (1, 2, 3)  # ×
  fuga: Tuple[int, int, int] = (1, 2, 3)  # ○
  

• 引数の型によって戻り値の型が変わる関数については@overloadデコレータを使用している

• Anyを使用していない

おわりに

こんな感じです。量と質のバランスが難しいですね。