Code Review · Python · Maintainability
Python Code Review: Deep Nesting
Review Python code where the main logic is buried under many nested if statements.
Code Review Exercise
Review the following function. The logic is not especially complex, but the structure makes it harder to read than it needs to be.
def apply_discount(user, cart, coupon):
if user is not None:
if user.is_active:
if cart is not None:
if len(cart.items) > 0:
if coupon is not None:
if coupon.is_valid:
if cart.total >= coupon.minimum_total:
cart.total = cart.total - coupon.amount
return True
else:
return False
else:
return False
else:
return False
else:
return False
else:
return False
else:
return False
else:
return FalseWhat review comments would you leave?
Review Findings
Issue #1: Deep nesting hides the main idea
The useful business rule is simple: apply the coupon only when the user, cart, coupon, and minimum total are valid. But the nested structure forces the reader to mentally track many levels of indentation before reaching the real action.
Issue #2: Many repeated return False branches
The function repeats return False in many places. This adds noise without adding clarity. A guard-clause style can make each failure condition explicit and remove most of the nesting.
Issue #3: Harder to modify safely
When code is deeply nested, adding one more condition often means inserting another level of indentation. Over time, this makes the method fragile and unpleasant to change.
Issue #4: Mixed validation and action
The function validates several conditions and also mutates the cart total. Separating the validation path from the action makes the code easier to review and test.
Improved Version: Guard Clauses
def apply_discount(user, cart, coupon):
if user is None or not user.is_active:
return False
if cart is None or not cart.items:
return False
if coupon is None or not coupon.is_valid:
return False
if cart.total < coupon.minimum_total:
return False
cart.total -= coupon.amount
return TrueThis version handles invalid cases early. The successful path is now easy to see at the bottom of the function.
Why This Is Better
- The code reads top-to-bottom.
- Each failure condition is clear.
- The main action is no longer buried inside many nested blocks.
- Future changes are less likely to create another indentation level.
Alternative: Extract a Predicate
If the validation rules grow, you can extract them into a helper so the main method reads like business logic.
def can_apply_coupon(user, cart, coupon):
if user is None or not user.is_active:
return False
if cart is None or not cart.items:
return False
if coupon is None or not coupon.is_valid:
return False
return cart.total >= coupon.minimum_total
def apply_discount(user, cart, coupon):
if not can_apply_coupon(user, cart, coupon):
return False
cart.total -= coupon.amount
return TrueThis Is Not Python-Specific
Deep nesting is a review smell in many languages: Python, Java, JavaScript, C#, Go, and others. The specific syntax changes, but the review comment is the same: make invalid cases explicit, reduce indentation, and keep the main path easy to follow.
Interview Follow-Ups
Are guard clauses always better?
No. They are useful when they make failure cases clearer. If overused, they can also scatter logic. The goal is readability, not blindly using a pattern.
Is deep nesting a correctness bug?
Not necessarily. It is usually a maintainability and readability problem. But hard-to-read code often leads to future correctness bugs.
What should you say in an interview?
Say that the code works in simple cases, but the nesting makes the success path hard to see. Then suggest guard clauses or extracted validation helpers.
When should logic be extracted into helper functions?
When a block has a clear name, is reused, or improves the main method's readability. Avoid extracting tiny helpers just to hide code.
What Interviewers Expect
A strong reviewer does not just say, "this is ugly." They explain why the structure creates risk.
- Readability: the main path is buried.
- Maintainability: adding conditions will make the method worse.
- Testing: validation rules are easier to test when separated.
- Refactoring: guard clauses or helpers can simplify the flow.