Code review and style

Before a change is merged, another programmer reviews it: reads it, asks questions and suggests improvements. Review finds bugs, but it also keeps the code readable for the next person.

What reviewers look for

1. Does it work? Is there a test for the new case?
2. Is it simple? Could it be shorter or clearer?
3. Are the names clear? Can you tell what a variable holds from its name?
4. Does it follow the project's style?
5. Is the documentation updated?

The same code, before and after review

def f(l):
    t = 0
    c = 0
    for x in l:
        if x >= 40:
            t = t + x
            c = c + 1
    if c == 0:
        return 0
    return t / c


print(f([35, 80, 55, 20, 90]))

This works, but what are f, l, t and c? What is 40? A reader has to work it all out.

PASS_MARK = 40


def average_of_passes(marks):
    """Return the average of the marks at or above PASS_MARK, or 0 if there are none."""
    passed = [mark for mark in marks if mark >= PASS_MARK]
    if not passed:
        return 0
    return sum(passed) / len(passed)


print(average_of_passes([35, 80, 55, 20, 90]))

The same result, with names that explain themselves, a named constant instead of a magic number, a docstring that says what the function returns, and a list comprehension instead of two counters.

Style guides and tools

Python's style guide is PEP 8: 4 spaces for indentation, snake_case for names, lines up to 79 characters, two blank lines between top-level functions. Most projects follow it, or say in CONTRIBUTING.md what they follow instead. You do not have to check it by eye:

• a formatter (black, ruff format) rewrites the layout for you;
• a linter (ruff, flake8, pylint) finds likely mistakes, such as unused variables and names used before they exist.

Run them before you push. Many projects run them in their checks too.

Giving and taking review

• Be kind and specific. "Could this be sum(passed) / len(passed)?" helps; "this is bad" does not.
• Explain why, and say which comments must be fixed and which are only suggestions (often marked "nit").
• The review is about the code, not you. Answer every comment, change what is right, and explain politely when you disagree.
• Say thank you. Reviewers are often volunteers.

Mark lesson complete