一键导入
python-anti-patterns
Common Python anti-patterns and a pre-merge review checklist.
用 Codex 或 Claude 帮你安装 复制这段 Prompt,粘贴到 Codex、Claude 或其他助手里,让它检查 Skill 页面并帮你完成安装。
菜单
Common Python anti-patterns and a pre-merge review checklist.
用 Codex 或 Claude 帮你安装 复制这段 Prompt,粘贴到 Codex、Claude 或其他助手里,让它检查 Skill 页面并帮你完成安装。
基于 SOC 职业分类
How agents, skills, and commands work in Claude Code projects.
GitHub conventions -- branch naming, commit format, issue/PR templates, and safe issue/PR referencing in comments.
API error response format -- machine-readable codes, human-readable reasons, status code rules.
Language-agnostic code hygiene -- honest comments, no dead/reinvented/duplicated code, truthful names, real implementations, and scoped (never blanket) diagnostic suppressions.
Language-agnostic structural craft -- decompose on responsibility not size, prefer deep modules over shallow piles, and shape cohesion, coupling, interfaces, error contracts, and data invariants. Sibling to code-hygiene and readable-code.
Documentation writing conventions -- style, structure, tone, and quality standards.
| name | python-anti-patterns |
| description | Common Python anti-patterns and a pre-merge review checklist. |
| when_to_use | Reviewing Python code, finalizing implementations, or debugging issues that might stem from known bad practices. |
A reference checklist of common mistakes and anti-patterns in Python code. Review this before finalizing implementations to catch issues early.
Note: This skill focuses on what to avoid. For guidance on positive patterns and architecture, see the python-patterns skill.
Timeouts, retries, and configuration should live in one place, not scattered across every call site.
Keep I/O, business logic, and API concerns in distinct layers. Don't mix SQL into business functions or leak ORM models to API consumers.
Catch specific exceptions, preserve partial results in batch operations, and validate inputs at boundaries.
Annotate all public functions, use typed collections, and let static analysis catch bugs before runtime.
# 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
Fix: Centralize in decorators or client wrappers.
# 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!
Fix: Retry at one layer only. Know your infrastructure's retry behavior.
# 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}...")
Fix: Use environment variables with typed settings.
# 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)
Fix: Use DTOs/response models.
# GOOD
@app.get("/users/{id}")
def get_user(id: str) -> UserResponse:
user = db.query(UserModel).get(id)
return UserResponse.from_orm(user)
# 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
Fix: Repository pattern. Keep business logic pure.
# 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
Fix: Catch specific exceptions. Log or handle appropriately.
# 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
Fix: Capture both successes and failures.
# 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
Fix: Validate early at API boundaries.
# 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?
Fix: Use context managers.
# 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!
Fix: Use async-native libraries.
# 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
Fix: Annotate all public functions.
# GOOD
def process(data: dict[str, int]) -> int:
return data["value"] * 2
# BAD: Generic list without type parameter
def get_users() -> list:
...
Fix: Use type parameters.
# 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
Fix: Test error conditions and edge cases.
# 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
Fix: Use integration tests for critical paths. Mock only external services.
# BAD: Default list is shared across all calls
def append_to(item, items=[]):
items.append(item)
return items
append_to(1) # [1]
append_to(2) # [1, 2] -- not [2]!
Fix: Use None and create a new object inside the function.
# GOOD
def append_to(item, items=None):
if items is None:
items = []
items.append(item)
return items
type() vs isinstance()# BAD: Fails for subclasses
if type(obj) == list:
process(obj)
Fix: Use isinstance, which respects inheritance.
# GOOD
if isinstance(obj, list):
process(obj)
== None vs is None# BAD: == can be overridden by __eq__
if value == None:
process()
Fix: Use the identity check is None.
# GOOD
if value is None:
process()
# BAD: Pollutes namespace, hides where names come from
from os.path import *
Fix: Import only what you need.
# GOOD
from os.path import join, exists
Before finalizing code, verify:
except Exception: passdef f(x=[]))isinstance() not type() for type checksis None not == Nonefrom module import *)| Anti-Pattern | Fix |
|---|---|
| Scattered retry logic | Centralized decorators |
| Hard-coded config | Environment variables + pydantic-settings |
| Exposed ORM models | DTO/response schemas |
| Mixed I/O + logic | Repository pattern |
| Bare except | Catch specific exceptions |
| Batch stops on error | Return BatchResult with successes/failures |
| No validation | Validate at boundaries with Pydantic |
| Unclosed resources | Context managers |
| Blocking in async | Async-native libraries |
| Missing types | Type annotations on all public APIs |
| Only happy path tests | Test errors and edge cases |
| Mutable default args | Use None + create inside function |
type() comparison | isinstance() |
== None | is None |
| Wildcard imports | Explicit named imports |