199 lines
5.4 KiB
Markdown
199 lines
5.4 KiB
Markdown
# Common Mistakes in Python
|
|
|
|
This file captures recurring non-idiomatic patterns, especially code that reads like Java or TypeScript in a Python costume.
|
|
|
|
## 1. Boolean-flag methods that hide two behaviors
|
|
|
|
### Smell
|
|
|
|
```python
|
|
def fetch_user(user_id: str, include_deleted: bool = False) -> User | None:
|
|
...
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
Boolean mode flags often mean one function is doing two jobs. They create call sites like `fetch_user(id, True)` that say almost nothing.
|
|
|
|
### Better shape
|
|
|
|
- split into separate functions when the behaviors are meaningfully different
|
|
- or promote the difference to a real enum or config object when there are several modes
|
|
|
|
## 2. Broad `except Exception:` without a boundary reason
|
|
|
|
### Smell
|
|
|
|
```python
|
|
try:
|
|
do_work()
|
|
except Exception:
|
|
return None
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
This erases recovery meaning. Mature libraries like HTTPX and Click use exception hierarchies so callers can catch broadly or narrowly depending on what they can recover from.
|
|
|
|
### Better shape
|
|
|
|
- catch the specific failures you can handle
|
|
- use broad catch points only at real process, API, or CLI boundaries
|
|
- translate internal failures into a clear outer error contract
|
|
|
|
### Source signals
|
|
|
|
- `httpx/_exceptions.py:74-90` defines `HTTPError` as a broad catch point.
|
|
- `httpx/_exceptions.py:107-160` then narrows request, transport, and timeout failures for recovery.
|
|
- `src/click/exceptions.py:35-111` ties exception subtypes to different exit codes and help output.
|
|
|
|
## 3. Hidden I/O in constructors or properties
|
|
|
|
### Smell
|
|
|
|
```python
|
|
class User:
|
|
def __init__(self, user_id: str):
|
|
self.profile = requests.get(...).json()
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
Object creation now performs surprising network I/O, which makes testing, lifetime, and failure handling muddy.
|
|
|
|
### Better shape
|
|
|
|
- keep I/O in explicit factory or boundary methods
|
|
- make resource acquisition visible
|
|
- separate data containers from loading logic
|
|
|
|
## 4. Import-time side effects
|
|
|
|
### Smell
|
|
|
|
```python
|
|
client = connect_to_db()
|
|
load_config_from_network()
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
Importing a module should usually define names, not secretly talk to the outside world. Import-time side effects make startup order brittle and tests unpredictable.
|
|
|
|
### Better shape
|
|
|
|
- move initialization into explicit startup paths
|
|
- create resources in app setup, dependency wiring, or main entrypoints
|
|
|
|
## 5. Mixing validation, transport, and business logic in one class
|
|
|
|
### Smell
|
|
|
|
```python
|
|
class OrderModel(BaseModel):
|
|
def save(self): ...
|
|
def send_webhook(self): ...
|
|
def render_html(self): ...
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
Boundary schema, persistence logic, and business behavior cannot evolve independently anymore.
|
|
|
|
### Better shape
|
|
|
|
- let Pydantic, attrs, or dataclasses own data-shape concerns
|
|
- keep boundary translation explicit
|
|
- split long-lived behavior into services or domain objects when needed
|
|
|
|
### Source signals
|
|
|
|
- `pydantic/main.py:253-264` treats model construction as input validation.
|
|
- `pydantic/main.py:455-519` makes dumping an explicit serialization step.
|
|
- `src/_pytest/timing.py:24-64` uses frozen dataclasses for small internal value objects.
|
|
|
|
## 6. Fake sync wrappers around async code
|
|
|
|
### Smell
|
|
|
|
```python
|
|
def get(url: str) -> Response:
|
|
return asyncio.run(async_get(url))
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
This breaks in existing event loops and hides real async lifetime and cancellation semantics.
|
|
|
|
### Better shape
|
|
|
|
- provide separate sync and async entrypoints when semantics differ
|
|
- keep them parallel, not secretly interchangeable
|
|
|
|
### Source signals
|
|
|
|
- `httpx/_client.py:594-661` and `httpx/_client.py:1307-1375` define separate `Client` and `AsyncClient` types.
|
|
- `httpx/_client.py:639-660` vs. `httpx/_client.py:1353-1374` keep the constructor shapes parallel while changing transport types.
|
|
- `examples/asyncio/async_orm.py:61-67` vs. `examples/inheritance/joined.py:93-120` show the same split in SQLAlchemy session usage.
|
|
|
|
## 7. Overuse of `Any`
|
|
|
|
### Smell
|
|
|
|
```python
|
|
def send(data: Any, options: Any) -> Any:
|
|
...
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
The API contract disappeared.
|
|
|
|
### Better shape
|
|
|
|
- use concrete types when the contract is specific
|
|
- use `Protocol` when callers care about capability
|
|
- use explicit unions or aliases for ergonomic flexibility
|
|
|
|
### Source signals
|
|
|
|
- `Lib/typing.py:2132-2157` frames `Protocol` around structural subtyping.
|
|
- `Lib/typing.py:2155-2157` warns that runtime protocol checks ignore signatures.
|
|
- `httpx/_client.py:639-660` shows rich public typing instead of `Any`-shaped parameters.
|
|
|
|
## 8. Accidental public APIs through file layout
|
|
|
|
### Smell
|
|
|
|
```python
|
|
from mypkg.utils import helper_a
|
|
from mypkg.impl_v2 import thing
|
|
```
|
|
|
|
### Why it is a smell
|
|
|
|
Internal module layout becomes the public contract by accident, which makes refactors painful.
|
|
|
|
### Better shape
|
|
|
|
- publish a deliberate import surface
|
|
- re-export supported names
|
|
- use `__all__` when the boundary should be explicit
|
|
|
|
### Source signals
|
|
|
|
- `src/pytest/__init__.py:6-80` builds a top-level facade over `_pytest.*` internals.
|
|
- `src/pytest/__init__.py:98-186` pins that facade with `__all__`.
|
|
- `httpx/__init__.py:1-12` and `httpx/__init__.py:29-106` do the same while also rewriting exported objects to appear under `httpx`.
|
|
|
|
## Heuristic
|
|
|
|
If the code:
|
|
|
|
- hides resource lifetime
|
|
- hides write boundaries
|
|
- hides which failures matter
|
|
- hides what the public API really is
|
|
|
|
…it is usually fighting Python instead of using it.
|