Software quality: coding standards, static analysis and code review

Robot software is unusually unforgiving. A null pointer in a web app produces an error page; in a robot it may leave a motor running. Defects also cost more the later they are found: a misread unit caught in code review costs minutes, the same defect found during field trials costs a day of travel and a frustrated customer, and found after an incident it can cost far more. This lesson covers the everyday practices that catch defects early and cheaply: coding standards, automatic formatting and linting, static analysis, and code review. Testing, the other half of quality, is the subject of lesson 4.

The cost of defects over time

Studies of software projects have long reported that the cost of fixing a defect grows the later it is found, with figures such as tenfold between design and testing often quoted. The exact multipliers vary widely and some classic numbers are disputed, but the direction is not in doubt for robots, where late defects require hardware time, test-site access and sometimes field visits. The practical lesson is to invest in the cheap early filters: clear requirements, readable code, automated checks on every commit and thoughtful review.

Coding standards

A coding standard is a written set of rules for how code is written in a project. Some rules are about style (naming, layout, comment format); these matter for readability but are best enforced by tools rather than argued about. Others are about safety and correctness: avoid raw owning pointers, initialize every variable, check every return value, never use floating-point equality for control decisions, state units in names. For C++ the C++ Core Guidelines, edited by Bjarne Stroustrup and Herb Sutter, are the most widely used foundation, and ROS 2 publishes its own code style guide built on them. For Python, PEP 8 covers style and PEP 484 type hints add checkable types. For safety-critical embedded C and C++, the MISRA guidelines and AUTOSAR C++14 rules, met in ROB-320, are much stricter and restrict the language to a subset that tools can analyze.

Formatters and linters

A formatter rewrites code into a standard layout automatically: clang-format for C++, black or ruff format for Python. Once a formatter runs on every commit, style debates disappear from review. A linter goes further, flagging suspicious constructs: unused variables, shadowed names, missing type hints, functions that are too long. ROS 2 packages commonly use the ament_lint family (ament_flake8, ament_cpplint, ament_uncrustify, ament_xmllint and others), which can run as part of the package's tests. Python projects increasingly use ruff, which combines linting and formatting very quickly, and mypy or pyright for type checking.

import ast, textwrap
# A tiny "linter": flag float equality comparisons and names without unit suffixes.
src = textwrap.dedent("""
def at_goal(dist, tol_m):
    return dist == 0.0
speed = 1.2
timeout_s = 0.5
""")
tree = ast.parse(src)
units = ("_m", "_s", "_mps", "_rad", "_hz", "_a", "_v")
for node in ast.walk(tree):
    if isinstance(node, ast.Compare) and any(isinstance(op, ast.Eq) for op in node.ops):
        if any(isinstance(c, ast.Constant) and isinstance(c.value, float) for c in node.comparators):
            print(f"line {node.lineno}: float compared with ==")
    if isinstance(node, ast.Assign):
        for t in node.targets:
            if isinstance(t, ast.Name) and not t.id.endswith(units):
                print(f"line {node.lineno}: '{t.id}' has no unit suffix")

line 4: 'speed' has no unit suffix
line 3: float compared with ==

Static analysis

Static analysis tools examine code without running it to find deeper problems: possible null dereferences, buffer overruns, use after free, data races, integer overflow, uninitialized reads, dead code and violations of coding rules. For C++, clang-tidy offers hundreds of checks, including the cppcoreguidelines and bugprone families; cppcheck is a lighter alternative; commercial tools such as Coverity, Polyspace and Parasoft add deeper analysis and certified rule sets for safety standards. Compiler warnings are the cheapest static analysis of all: building with -Wall -Wextra and treating warnings as errors in continuous integration catches many problems for free.

Dynamic analysis complements static analysis by instrumenting the running program: AddressSanitizer finds memory errors, ThreadSanitizer finds data races, UndefinedBehaviorSanitizer finds undefined behavior, and Valgrind finds leaks. These sanitizers are best run in continuous integration on the test suite, because they slow programs down too much for normal use.

Every static analyzer produces false positives, warnings about code that is actually fine. A tool that cries wolf is soon ignored, so teams tune the rule set, suppress specific warnings with a written justification next to the code, and require that new code introduce no new warnings. The ratio of real defects to warnings, the precision, is worth tracking.

Code review

Code review means that every change is read by at least one other engineer before it is merged. Most teams do it through pull requests (or merge requests) on a hosting service, with automated checks running first so that reviewers spend their attention on what tools cannot judge: whether the change does what it claims, whether the design fits the architecture, whether edge cases and failure modes are handled, whether tests are adequate and whether the code will be understandable in a year.

Research and industry experience suggest that review is most effective when changes are small and reviewers are not rushed. Reviewing a few hundred lines at a time, for no more than about an hour, finds more defects per line than reviewing thousands of lines at once. Authors help by writing a clear description of what changed and why, linking the requirement or issue, and pointing out the parts they are least sure about. Reviewers help by commenting on the code rather than the person, distinguishing blocking issues from suggestions, and approving promptly when concerns are met.

Ownership, branching and change control

Quality also depends on how changes flow. Most robot teams use trunk-based development or short-lived feature branches merged frequently into a main branch that is always buildable, with protected branch rules requiring passing checks and approval before merge. CODEOWNERS files assign default reviewers for each directory, so that a change to the safety monitor automatically requests review from the safety lead. Release branches, tags and changelogs (lesson 7) give a controlled path from main to robots in the field. For safety-relevant code, change control may also require a documented impact analysis: which requirements and tests are affected, which hazards in the risk assessment might change.

Measuring quality without gaming it

Teams track metrics such as test coverage, static-analysis warnings, open defects, review turnaround, escaped defects (found after release) and mean time to restore service after a failure. Metrics are useful as trends and alarms but harmful as targets: a team pushed to raise coverage will write tests that execute code without checking it. The most telling metric for a robot team is usually escaped defects per release and their severity, because it measures what the customer actually experiences.