A reference checklist of common mistakes and anti-patterns in Python code. Review this before finalizing implementations to catch issues early. - Reviewing code before merge - Debugging mysterious issues
python-design-patterns skill.# 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) `### Double Retry` # BAD: Retrying at multiple layers @retry(max_attempts=3) # Application retry def call_service(): return client.request() # Client also has retry configured!
# 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) `### Mixed I/O and Business Logic` # 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)) `### Ignored Partial Failures` # 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) `### Missing Input Validation` # 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() `### Blocking in Async` # 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 `### Untyped Collections` # 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) `### Over-Mocking` # 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
except Exception: pass