Code Review

Two Programs, Only One Correct

Look at these two versions of a function. Both are meant to do the exact same job: take a list of a student's marks and return the average. Both run without crashing on the test case shown. Read them and decide — are they both correct?

def average_marks(marks):
    total = 0
    for m in marks:
        total = total + m
    average = total / len(marks)
    return average

print(average_marks([80, 90, 70]))

Run it, and you get 80.0. That looks right — (80 + 90 + 70) / 3 is indeed 80. If this were the only test you ran, you would ship this code and move on. But watch what happens the day a student is absent for a test and the teacher's roster passes an empty list, because there are no marks to average yet:

print(average_marks([]))
# ZeroDivisionError: division by zero

The program crashes. Not because the logic for computing an average is wrong — it's algebraically correct — but because nobody checked what happens when the input is a case the "obvious" test never covered. This is the entire reason code review exists as a separate skill from writing code. A program can run, produce a plausible-looking number, and still be broken. Writing code answers the question "does this do what I meant?" Reviewing code answers a harder, more important question: "does this do what I meant, for every input it will ever see, and will the next person who reads it understand why?"

What Code Review Actually Means

Code review is the practice of a person other than the author reading a piece of code — carefully, line by line — before it is accepted, in order to catch mistakes, question unclear logic, and suggest improvements that the author, being close to their own work, is likely to miss. It is not testing (running the program to see its output) and it is not debugging (fixing a program you already know is broken). Code review happens by reading, often before the code is even run on the case that would reveal the bug. A good reviewer can look at the average_marks function above and predict the ZeroDivisionError without ever executing it, simply by asking: "what if marks is empty?"

This matters more than it might seem. In any real software system — a UPI payment app, an IRCTC ticket-booking backend, a school's result-management portal — thousands of lines of code get written every week, often by many different programmers. If each person's code goes live the moment it "runs on my machine," bugs like the one above reach real users instead of being caught by a colleague first. Code review is the checkpoint that sits between "I wrote this" and "this is now part of the product everyone uses."

Misconception: "It Runs Without an Error" Does Not Mean "It Is Correct"

This is the single most common misunderstanding among students learning to program, so it is worth stating precisely. A program can fail in three very different ways, and only one of them announces itself loudly:

• It crashes. Python prints a traceback and stops, like the ZeroDivisionError above. This is actually the easiest kind of bug, because the program tells you exactly where and why it failed.
• It runs to completion but gives the wrong answer. No error message appears. The output looks like a number, a list, a grade — something plausible. This is far more dangerous, because nothing alerts you that anything is wrong. You have to already suspect a mistake to go looking for one.
• It works on the inputs you happened to test and fails on inputs you didn't think to try. This is what happened with the empty list above — the code was tested on one "normal" case and never tested on the edge case.

A programmer testing their own code tends to test the cases they already imagined while writing it — which is exactly why those cases pass. A reviewer, coming to the code fresh, is more likely to imagine the cases the author didn't. That outside perspective, not superior skill, is the main reason code review catches bugs that the original author's own testing missed.

A Reviewer Walks Through Real Code

Let's do what a reviewer actually does: read the average_marks function line by line and narrate the questions a careful reader asks at each step.

def average_marks(marks):
    total = 0
    for m in marks:
        total = total + m
    average = total / len(marks)
    return average

Line 1 — def average_marks(marks): Reasonable name; it says what the function returns. No docstring, though — a reviewer would ask: what should this return if marks is empty? The function's behaviour for that case is not documented anywhere, which is itself a review comment worth raising even before finding the bug.

Lines 2–4 — the summing loop. Correct: it adds every element of marks into total. No issue here.

Line 5 — average = total / len(marks). This is where a reviewer's eye should catch on len(marks) appearing in a denominator. Any time a variable's length (or any quantity that could be zero) is used as a divisor, that is a signal to ask: "can this ever be zero, and if so, what happens?" Here, yes — an empty marks list makes len(marks) equal to 0, and dividing by zero crashes the program.

The fix a reviewer would request is small but essential — a guard clause that handles the empty case explicitly, plus a docstring that states the contract of the function so future readers don't have to guess:

def average_marks(marks):
    """Return the average of a list of marks.
    Returns None if the list is empty."""
    if len(marks) == 0:
        return None
    total = 0
    for m in marks:
        total = total + m
    average = total / len(marks)
    return average

Notice what changed and what didn't. The summing logic — the part the author was actually thinking hard about — was already correct and needed no change. The bug was in a part the author probably never consciously thought about at all: the boundary case of an empty input. That is a pattern you will see again and again in code review — bugs cluster at edges (empty lists, zero, the first element, the last element), not in the "main" logic.

The Second Bug: Off-by-One in a Loop

Here is a different function, written by a student to find the highest score in a list of exam scores:

def highest_score(scores):
    highest = scores[0]
    for i in range(len(scores) - 1):
        if scores[i] > highest:
            highest = scores[i]
    return highest

scores = [45, 67, 58, 92]
print(highest_score(scores))

Before reading further, trace it yourself: scores = [45, 67, 58, 92] has length 4, so range(len(scores) - 1) is range(3), which produces i = 0, 1, 2 — never i = 3. The loop checks scores[0]=45, scores[1]=67, and scores[2]=58, updating highest to 67 when it sees index 1. It never looks at scores[3], which holds 92 — the actual highest value. The function prints 67, which is wrong.

This class of mistake is called an off-by-one error: the loop's boundary is one position short of (or one position past) where it should be. It is one of the most common bugs in all of programming, and it is very easy for the original author to miss, because range(len(scores) - 1) "looks like" it should visit every index — the - 1 feels defensive, as if it's protecting against something, rather than what it actually does, which is silently discard the last element every single time.

The fix is to let the loop run across every valid index, 0 through len(scores) - 1 inclusive, which is exactly what range(len(scores)) produces:

def highest_score(scores):
    highest = scores[0]
    for i in range(len(scores)):
        if scores[i] > highest:
            highest = scores[i]
    return highest

print(highest_score(scores))
# 92

Trace it again: i = 0,1,2,3. scores[0]=45 (no change), scores[1]=67>45 (highest becomes 67), scores[2]=58 (no change), scores[3]=92>67 (highest becomes 92). The function now correctly returns 92. A thorough reviewer might add one more comment here, not about correctness but about clarity: Python's built-in max(scores) does exactly this in one call, and using it would remove the possibility of this bug ever existing at all. That is a completely valid review comment — not every fix is about correctness; some are about writing code that is harder to get wrong in the first place.

What a Code Review Looks Like

In real teams, reviewers don't just think these comments — they write them directly next to the specific line of code that caused the concern, so the author can see exactly what triggered it. The diagram below shows what that looks like for the highest_score bug: the red line is what the author submitted, the green line is the reviewer's suggested fix, and the callout is the actual comment a reviewer would leave.

Notice the shape of that comment: it does not just say "this is wrong." It states the specific input that breaks it ([45, 67, 58, 92]), the specific wrong output it produces (67 instead of 92), and a concrete suggested fix. That is what separates a useful review comment from an unhelpful one — "this looks buggy" gives the author nothing to act on, while a comment with a concrete failing example lets them verify the bug in seconds and know exactly what to change.

A Reviewer's Checklist

Once you have seen a few real bugs get caught this way, the questions a reviewer asks stop feeling random and start falling into a small number of categories. Every serious code review, whether done by a classmate on a school project or by an engineer at a company, is really checking these things:

• Correctness: Does the code produce the right output for the cases it was designed for? This is checked by tracing the logic by hand, the way we traced highest_score above, not just by trusting that it "looks right."
• Edge cases: What happens at the boundaries — an empty list, a list with one element, the number zero, a negative number, the very first or very last position in a loop? Most real bugs, including both bugs in this chapter, live at edges, not in the middle of the "normal" logic.
• Naming and readability: Do variable and function names say what they hold or do? A variable named x in a five-line function is harmless; a variable named x in a fifty-line function forces every future reader to scroll back and forth to remember what it means. Good names are a form of documentation that never goes out of date.
• Duplication (the "DRY" principle — Don't Repeat Yourself): If the same three lines of logic appear in two different functions, that is a review flag. Not because duplicate code is wrong today, but because the day someone fixes a bug in one copy and forgets the other, the two copies silently disagree, and nothing tells you which one is correct anymore.
• Efficiency: Is there a needlessly slow way of doing something that has an equally simple, faster alternative — like writing a manual loop to find a maximum when max() already does exactly that, correctly, in one call?

A useful way to remember the order to check these in: correctness and edge cases first, because a beautifully readable function that gives the wrong answer is still useless. Readability and duplication come after — they matter for the long-term health of the code, but they don't matter if the code doesn't work in the first place.

Misconception: Code Review Is Not Criticism of the Person

A second misconception, especially common the first time students review each other's work, is treating a comment on the code as a comment on the coder. It is easy for "this function crashes on an empty list" to be heard as "you are a careless programmer," even though that was never what was said. Professional reviewers avoid this trap through a habit of language, not through pretending mistakes don't matter: they phrase comments about the code, not the author, and they distinguish between a required fix and a suggestion.

Compare "You forgot to handle empty lists, this is sloppy" with "This crashes when marks is empty — worth adding a guard clause for that case." Both point at the same bug. Only one of them is useful to receive, because it describes the code's behaviour and proposes a next step, instead of judging the person who wrote it. Reviewers also commonly label comments by how serious they are — a small style preference gets prefixed with something like nit: (short for "nitpick," meaning "take this or leave it, it's minor"), while a real bug is stated as a blocking issue that must be fixed before the code is accepted. That labelling lets the author immediately tell the difference between "please rename this variable if you get a chance" and "this will crash in production, fix it before merging."

How This Works on Real Software Teams

On professional teams, code review is not optional and not informal — it is a required step built into how code moves from a programmer's laptop into the live product. A programmer writes a change, submits it for review (commonly called a "pull request"), and one or more teammates read the change before it is allowed to merge into the main codebase that the product actually runs. The reviewer can approve it, or "request changes," which sends it back to the author with comments exactly like the one in the diagram above. Nothing reaches real users — the app processing your UPI payment, the portal booking your IRCTC ticket, the system generating your school's report cards — without at least one other person having read it first and confirmed it does what it claims to do, including on the edge cases the original author didn't think to test. The average_marks bug in this chapter is a small, classroom-scale version of exactly the kind of mistake that code review exists to catch before it reaches a system millions of people rely on.

This same skill — reading code closely enough to spot exactly where and why it fails — is also directly tested in Computer Science exams: a "find the bug" or "predict the output" question asks you to do precisely what a reviewer does, trace the code by hand and identify the line that produces the wrong result. Practising code review is, quite literally, practising for that question type.

Try It Yourself: Review These Three Snippets

Before checking the notes below each one, read every snippet the way a reviewer would: trace it by hand, decide what it should do, and find where it disagrees with that.

def is_eligible_for_scholarship(percentage):
    if percentage > 90:
        return True
    else:
        return False

print(is_eligible_for_scholarship(90))

Snippet A. The scholarship rule is "90% or above qualifies." Trace is_eligible_for_scholarship(90): the condition is 90 > 90, which is False, so the function returns False — a student with exactly 90% is wrongly rejected. This is a boundary bug: the comparison should be >=, not >. It is the same category of mistake as the empty-list bug, just at a numeric boundary instead of a list boundary.

def calc1(a, b):
    c = a + b
    d = c / 2
    return d

def calc2(x, y):
    z = x + y
    w = z / 2
    return w

Snippet B. Trace both: each one adds its two arguments and divides by 2 — they are the average of two numbers, written twice under two different names with meaningless variable names (c, d, z, w say nothing about what they hold). A reviewer would flag two separate issues here: duplication (delete one function and call the other, or better, name it clearly as average_of_two(a, b) and use it everywhere), and naming (rename the working variable to something like total and the result to average).

def get_last_mark(marks):
    return marks[len(marks)]

Snippet C. Trace it with marks = [78, 85, 91]: len(marks) is 3, and marks[3] is requested — but valid indices for a 3-element list are only 0, 1, and 2. This raises IndexError: list index out of range. The fix is marks[len(marks) - 1], or more simply, Python's negative indexing: marks[-1], which always means "the last element" regardless of the list's length.

Summary

Code review is the practice of reading someone else's code — carefully, before it is accepted — to catch mistakes the author's own testing missed, because the author naturally tests the cases they already imagined while writing it. A program that runs without an error message is not the same thing as a program that is correct: it may crash only on inputs nobody tried, or it may run cleanly and simply return the wrong answer, as highest_score did by silently ignoring the last element of its list. The bugs that survive an author's own testing tend to live at edges — empty inputs, zero, the first or last position — which is exactly why edge cases sit near the top of a reviewer's checklist, alongside correctness, naming, duplicated logic, and efficiency. A good review comment names the specific input that breaks the code, the specific wrong output it produces, and a concrete suggested fix — and it is phrased as feedback about the code, never as a judgement of the person who wrote it. This is not a classroom formality: on real software teams, no change reaches the product people actually use without a reviewer reading it first, and the exact skill of tracing code to find where it disagrees with what it should do is also the skill Computer Science exams test directly when they ask you to debug or predict output.