Avoid Common Python Code Smells

A code smell is a surface-level pattern that suggests a deeper design problem. It is not a bug yet, but it makes the code harder to understand, harder to test, and more likely to hide real bugs. Learning to recognize these patterns helps you write cleaner code and catch problems during code review.

This article covers the most common Python-specific smells that appear in real codebases. Each section shows the problematic pattern, explains why it causes trouble, and gives the clean alternative. These habits complement the naming advice in Name Python Variables, Functions, and Classes.

Mutable default arguments

The most famous Python gotcha. Default argument values are evaluated once when the function is defined, not each time the function is called. If the default is a mutable object like a list or dict, every call shares the same object:

python

# Smell: mutable default argument
def add_task(task, tasks=[]):
    tasks.append(task)
    return tasks
 
print(add_task("write tests"))   # ['write tests']
print(add_task("fix bug"))       # ['write tests', 'fix bug']  -- unexpected!

The second call reuses the same list that the first call modified. This leads to subtle bugs that only appear after multiple calls.

The fix uses None as the default and creates a new mutable object inside the function body:

python

# Clean: immutable default with explicit creation
def add_task(task, tasks=None):
    if tasks is None:
        tasks = []
    tasks.append(task)
    return tasks

This pattern works for lists, dictionaries, sets, and any other mutable type. It is explicit about the intent: create a fresh collection for each call that does not provide one.

Modifying a list while iterating

Removing items from a list while looping over it causes the loop to skip elements. When you delete an item at index i, the next item shifts into position i, but the loop advances to i+1 and misses it:

python

# Smell: modifying during iteration
numbers = [1, 2, 3, 4, 5, 6]
for num in numbers:
    if num % 2 == 0:
        numbers.remove(num)
# Result: [1, 3, 5] -- but only by accident, skips items

The clean solution creates a new list with only the items you want to keep. A list comprehension says the intent directly and avoids mutation during iteration:

python

# Clean: build a new list
numbers = [1, 2, 3, 4, 5, 6]
numbers = [num for num in numbers if num % 2 != 0]

If you need to modify a list in place due to memory constraints, iterate over a copy using slice notation for num in numbers[:]:. But the comprehension approach is preferred because it avoids side effects entirely.

Boolean trap arguments

A function that accepts a boolean flag to change its behavior is harder to read at the call site. When you see fetch_data(url, True), the reader has no idea what True means without looking up the function definition:

python

# Smell: boolean trap
def fetch_data(url, use_cache):
    if use_cache:
        return read_from_cache(url)
    return download(url)
 
result = fetch_data("/api/users", True)  # what does True mean?

Replace the boolean with two separate functions, or use keyword-only arguments with descriptive names. The call site should read like a sentence:

python

# Clean: separate functions
def fetch_data(url):
    return download(url)
 
def fetch_data_cached(url):
    return read_from_cache(url) or download(url)

If combining them makes sense, use an enum or string constant instead of a boolean. Define a small enum class that names each mode clearly. The call site then reads like a sentence:

python

from enum import Enum
 
class CachePolicy(Enum):
    ENABLED = "enabled"
    DISABLED = "disabled"
 
def fetch_data(url, cache_policy=CachePolicy.ENABLED):
    if cache_policy == CachePolicy.ENABLED:
        cached = read_from_cache(url)
        if cached is not None:
            return cached
    return download(url)

The caller writes fetch_data(url, CachePolicy.DISABLED) and the intent is obvious without looking up the function signature.

Type-checking with type() instead of isinstance()

Using type(obj) == SomeClass does not account for inheritance. A subclass instance will fail the check even though it behaves like the parent class in every way. This breaks polymorphism and makes code fragile:

python

# Smell: strict type equality
if type(value) == dict:
    process_dict(value)

Use isinstance() instead. It returns True for the specified class and any subclass, which respects Python's object model and keeps polymorphism working the way callers expect:

python

# Clean: isinstance respects inheritance
if isinstance(value, dict):
    process_dict(value)

The only valid use of type() comparison is when you genuinely need an exact type match, such as avoiding infinite recursion in a custom repr method. Those cases are rare.

Using a bare except clause

A bare except catches everything, including KeyboardInterrupt and SystemExit. This can prevent a program from shutting down. Always catch a specific exception type. The first version below is dangerous; the second is safe:

python

# Smell: bare except
try:
    result = risky_operation()
except:
    logger.error("Something went wrong")
 
# Clean: specific exception
try:
    result = risky_operation()
except (ValueError, ConnectionError) as error:
    logger.error("Operation failed: %s", error)

At minimum use except Exception if you genuinely need to catch all runtime errors, but even that should be a rare last resort in production error handlers.

Overusing global variables

Global variables create hidden coupling between functions. Any function in the module can read or modify the global, making it impossible to reason about state locally. Tests become unreliable because one test's modifications affect the next test:

python

# Smell: global mutable state
config = {}
 
def load_config():
    global config
    config = read_file("config.toml")

Pass state explicitly through function arguments and return values instead. This makes dependencies visible in the function signature and lets each function be tested in isolation without shared setup:

python

# Clean: explicit parameter passing
def load_config(path):
    return read_file(path)
 
def process(config):
    return config.get("mode", "default")

Module-level constants, read-only values that never change after import, are fine. The problem is mutable global state that different parts of the program modify from different places.

Reinventing built-in functionality

Python has a rich standard library. Before writing a helper for something that feels like a common operation, check whether a built-in function or standard library module already handles it:

• Custom search loops replaced by any() and all()
• Manual min/max loops replaced by min() and max()
• Index-based lookups replaced by dict.get() with a default
• Hand-rolled CSV parsing replaced by the csv module

Python's standard library has tools for most common tasks. Using them means less code to write, test, and debug.

Replacing a hand-rolled helper with a standard library call removes code you no longer need to test or debug. For more on writing clean, maintainable Python, see Write Clean and Readable Python Code.