python-anti-patterns について
Python 開発者向けのコードレビュー用リファレンススキルです。マージ前の確認、デバッグ、リファクタリングでよくある実装上の問題を見つけるため、分野別のアンチパターンと改善例、レビュー用チェックリストをまとめています。
主な機能
- 01分野別のチェックリスト
- 02問題例と改善例
- 03エラー処理とリソース管理の指針
- 04非同期処理と型安全性の注意点
- 05Python 固有の落とし穴の解説
活用シーン
- 01マージ前の Python コードレビュー
- 02繰り返す実行時問題の調査
- 03Python の実践知識の共有やチーム標準の策定
- 04リファクタリング時のレガシーコード評価
インストール
コマンドをコピーしてターミナルで実行します。
npx skills add https:/ / github.com/ julianobarbosa/ claude-code-skills --skill python-anti-patternsインストール後、Claude Code で/python-anti-patternsを入力して読み込まれたか確認します。
詳細
Python のアンチパターンチェックリスト
Python コードでよくある間違いやアンチパターンをまとめたチェックリストです。実装を確定する前に確認して、問題を早期に見つけましょう。
このスキルを使う場面
- マージ前のコードレビュー
- 原因不明の問題のデバッグ
- Python のベストプラクティスの指導や学習
- チームのコーディング規約の策定
- レガシーコードのリファクタリング
注: このスキルでは、避けるべきことに焦点を当てています。望ましいパターンやアーキテクチャについては、python-design-patterns スキルを参照してください。
インフラストラクチャのアンチパターン
タイムアウト/リトライ処理の分散
# BAD: Timeout logic duplicated everywhere
def fetch_user(user_id):
try:
return requests.get(url, timeout=30)
except Timeout:
logger.warning("Timeout fetching user")
return None
def fetch_orders(user_id):
try:
return requests.get(url, timeout=30)
except Timeout:
logger.warning("Timeout fetching orders")
return None
修正方法: デコレーターまたはクライアントラッパーに一元化します。
# GOOD: Centralized retry logic
@retry(stop=stop_after_attempt(3), wait=wait_exponential())
def http_get(url: str) -> Response:
return requests.get(url, timeout=30)
二重リトライ
# BAD: Retrying at multiple layers
@retry(max_attempts=3) # Application retry
def call_service():
return client.request() # Client also has retry configured!
修正方法: リトライは1つのレイヤーだけで行います。インフラストラクチャのリトライ動作を把握しておきましょう。
設定値のハードコード
# BAD: Secrets and config in code
DB_HOST = "prod-db.example.com"
API_KEY = "sk-12345"
def connect():
return psycopg.connect(f"host={DB_HOST}...")
修正方法: 型付き設定と環境変数を使用します。
# GOOD
from pydantic_settings import BaseSettings
class Settings(BaseSettings):
db_host: str = Field(alias="DB_HOST")
api_key: str = Field(alias="API_KEY")
settings = Settings()
アーキテクチャのアンチパターン
内部型の公開
# BAD: Leaking ORM model to API
@app.get("/users/{id}")
def get_user(id: str) -> UserModel: # SQLAlchemy model
return db.query(UserModel).get(id)
修正方法: DTO/レスポンスモデルを使用します。
# GOOD
@app.get("/users/{id}")
def get_user(id: str) -> UserResponse:
user = db.query(UserModel).get(id)
return UserResponse.from_orm(user)
I/O とビジネスロジックの混在
# BAD: SQL embedded in business logic
def calculate_discount(user_id: str) -> float:
user = db.query("SELECT * FROM users WHERE id = ?", user_id)
orders = db.query("SELECT * FROM orders WHERE user_id = ?", user_id)
# Business logic mixed with data access
if len(orders) > 10:
return 0.15
return 0.0
修正方法: リポジトリパターンを使い、ビジネスロジックは純粋に保ちます。
# GOOD
def calculate_discount(user: User, orders: list[Order]) -> float:
# Pure business logic, easily testable
if len(orders) > 10:
return 0.15
return 0.0
エラー処理のアンチパターン
例外の無条件な捕捉
# BAD: Swallowing all exceptions
try:
process()
except Exception:
pass # Silent failure - bugs hidden forever
修正方法: 特定の例外を捕捉し、適切にログを記録するか処理します。
# GOOD
try:
process()
except ConnectionError as e:
logger.warning("Connection failed, will retry", error=str(e))
raise
except ValueError as e:
logger.error("Invalid input", error=str(e))
raise BadRequestError(str(e))
部分的な失敗の無視
# BAD: Stops on first error
def process_batch(items):
results = []
for item in items:
result = process(item) # Raises on error - batch aborted
results.append(result)
return results
修正方法: 成功と失敗の両方を記録します。
# GOOD
def process_batch(items) -> BatchResult:
succeeded = {}
failed = {}
for idx, item in enumerate(items):
try:
succeeded[idx] = process(item)
except Exception as e:
failed[idx] = e
return BatchResult(succeeded, failed)
入力検証の不足
# BAD: No validation
def create_user(data: dict):
return User(**data) # Crashes deep in code on bad input
修正方法: API の境界で早期に検証します。
# GOOD
def create_user(data: dict) -> User:
validated = CreateUserInput.model_validate(data)
return User.from_input(validated)
リソースのアンチパターン
リソースを閉じ忘れる
# BAD: File never closed
def read_file(path):
f = open(path)
return f.read() # What if this raises?
修正方法: コンテキストマネージャーを使用します。
# GOOD
def read_file(path):
with open(path) as f:
return f.read()
非同期コード内でのブロッキング
# BAD: Blocks the entire event loop
async def fetch_data():
time.sleep(1) # Blocks everything!
response = requests.get(url) # Also blocks!
修正方法: 非同期処理に対応したライブラリを使用します。
# GOOD
async def fetch_data():
await asyncio.sleep(1)
async with httpx.AsyncClient() as client:
response = await client.get(url)
型安全性のアンチパターン
型ヒントの不足
# BAD: No types
def process(data):
return data["value"] * 2
修正方法: すべての公開関数に型注釈を付けます。
# GOOD
def process(data: dict[str, int]) -> int:
return data["value"] * 2
型指定のないコレクション
# BAD: Generic list without type parameter
def get_users() -> list:
...
修正方法: 型パラメーターを使用します。
# GOOD
def get_users() -> list[User]:
...
テストのアンチパターン
正常系しかテストしない
# BAD: Only tests success case
def test_create_user():
user = service.create_user(valid_data)
assert user.id is not None
修正方法: エラー条件と境界ケースをテストします。
# GOOD
def test_create_user_success():
user = service.create_user(valid_data)
assert user.id is not None
def test_create_user_invalid_email():
with pytest.raises(ValueError, match="Invalid email"):
service.create_user(invalid_email_data)
def test_create_user_duplicate_email():
service.create_user(valid_data)
with pytest.raises(ConflictError):
service.create_user(valid_data)
モックの使いすぎ
# BAD: Mocking everything
def test_user_service():
mock_repo = Mock()
mock_cache = Mock()
mock_logger = Mock()
mock_metrics = Mock()
# Test doesn't verify real behavior
修正方法: 重要な処理経路には統合テストを使用します。モックは外部サービスに限定します。
簡易レビュー用チェックリスト
コードを確定する前に、次の項目を確認してください。
- タイムアウト/リトライ処理が分散していない(一元化されている)
- 二重リトライがない(アプリケーション+インフラストラクチャ)
- 設定値やシークレットがハードコードされていない
- 内部型(ORM モデル、protobuf)が公開されていない
- I/O とビジネスロジックが混在していない
-
except Exception: passのような無条件な例外処理がない - バッチ処理で部分的な失敗を無視していない
- 入力検証が不足していない
- リソースを閉じ忘れていない(コンテキストマネージャーを使用している)
- 非同期コード内にブロッキング呼び出しがない
- すべての公開関数に型ヒントがある
- コレクションに型パラメーターが指定されている
- エラー経路がテストされている
- 境界ケースがカバーされている
よくある修正方法のまとめ
| アンチパターン | 修正方法 |
|---|---|
| リトライ処理の分散 | デコレーターに一元化 |
| 設定値のハードコード | 環境変数+pydantic-settings |
| ORM モデルの公開 | DTO/レスポンススキーマ |
| I/O とロジックの混在 | リポジトリパターン |
| 無条件な except | 特定の例外を捕捉 |
| エラー時にバッチ処理が停止する | 成功/失敗を含む BatchResult を返す |
| 検証がない | Pydantic を使って境界で検証 |
| リソースを閉じ忘れる | コンテキストマネージャー |
| 非同期コード内でのブロッキング | 非同期処理に対応したライブラリ |
| 型指定の不足 | すべての公開 API に型注釈を付ける |
| 正常系しかテストしない | エラーと境界ケースをテスト |
注意点
- ミュータブルなデフォルト引数は呼び出し間で状態を共有します:
def f(x=[]):の場合、xを指定しないすべての呼び出しで同じリストにアクセスします。x=Noneを使い、関数内で作成してください。 - 裸の
except:やexcept Exception:は、BaseException経由でのみKeyboardInterruptを捕捉します — 裸の except を使った長時間実行ループは、Ctrl+C を黙って無視します。 __slots__はメモリを節約しますが、必要な箇所で__weakref__と__dict__のエントリーを正確に宣言しないと、pickle 化、weakref、多重継承で問題が発生します。- キャッシュされた小さな整数/文字列に対する
isと==の違い — 小さな整数(-5..256)やインターンされた文字列ではa is bが機能しますが、それ以外では予測できない結果になります。値の比較には常に==を使ってください。 python -Oではassertが取り除かれます — 実行時の検証には使わず、コード内部の不変条件にのみ使用してください。