Code Review · Python

Python Code Review: Hidden Side Effects

Review Python code that unexpectedly mutates a caller's list and dictionaries while appearing to return cleaned data.

Code Review Exercise

Review the following Python function. It looks like it prepares user records for display, but there is an important design issue hidden in the implementation.

python
def normalize_users(users):
    for user in users:
        user["email"] = user["email"].lower().strip()
        user["name"] = user["name"].strip()

    users.sort(key=lambda user: user["name"])
    return users


raw_users = [
    {"name": "  Bob", "email": "BOB@example.com "},
    {"name": "Alice  ", "email": " ALICE@example.com"},
]

clean_users = normalize_users(raw_users)

print(raw_users)
print(clean_users)

What concerns do you see before scrolling further?

Review Findings

Issue #1: The function mutates the caller's data

The function changes each dictionary inside users in place. A caller might expect normalize_users to return cleaned data without changing the original input.

Issue #2: sort() also mutates the input list

users.sort(...) reorders the same list object that was passed into the function. This is a hidden side effect because the caller's list order changes even if the caller only uses the return value.

Issue #3: The function contract is unclear

The name normalize_users does not make it obvious whether the function mutates input or returns a new normalized list. In review, this is worth calling out because unclear contracts create bugs later.

Issue #4: Shared nested objects can still leak changes

Even if the list were copied with users.copy(), the dictionaries inside the list would still be shared. For nested mutable structures, a shallow copy may not be enough.

Why This Happens

In Python, lists and dictionaries are mutable objects. When a list is passed to a function, the function receives a reference to the same list object, not a separate copy.

That means operations like append(), sort(), item assignment, and dictionary updates can change data that the caller still holds.

This is not always wrong. Sometimes mutation is intentional. The problem is when the mutation is surprising or undocumented.

Fixed Version: Return New Objects

python
def normalize_users(users):
    normalized = []

    for user in users:
        normalized.append({
            "name": user["name"].strip(),
            "email": user["email"].lower().strip(),
        })

    return sorted(normalized, key=lambda user: user["name"])

This version creates new dictionaries and returns a new sorted list. The original input remains unchanged.

Alternative: Make Mutation Explicit

python
def normalize_users_in_place(users):
    for user in users:
        user["email"] = user["email"].lower().strip()
        user["name"] = user["name"].strip()

    users.sort(key=lambda user: user["name"])

If mutation is intentional, the function name should say so. The _in_place suffix makes the side effect much harder to miss during code review.

Interview Follow-Ups

Is mutating input always bad?

No. Mutating input can be fine when it is intentional, documented, and expected by callers. The issue is hidden mutation.

What is the difference between sorted() and list.sort()?

sorted() returns a new sorted list. list.sort() sorts the existing list in place and returns None.

Is users.copy() enough here?

Not fully. It copies the outer list, but the dictionaries inside are still shared. Updating a dictionary would still affect the original data.

What should a reviewer ask?

Ask whether the function is intended to mutate the input. If yes, make that explicit. If no, return new objects instead.

What Interviewers Expect

A strong reviewer notices that the function is technically correct for one caller, but dangerous as a reusable utility.

  • Correctness: caller data changes unexpectedly.
  • Maintainability: the function contract is unclear.
  • Readability: mutation should be obvious from the name or docs.
  • Design: prefer returning new data unless mutation is intentional.