Review a small function with a real specification
The project is a fictional checkout calculator, not a payment processor. It accepts an integer subtotal in cents, a discount rate in basis points and an integer delivery charge in cents. One hundred basis points is one percentage point; the allowed discount is zero through 10,000 basis points. Subtotal and delivery must be nonnegative integers, and Boolean values are rejected rather than treated as zero or one.
The contract discounts the subtotal, rounds the discount to the nearest cent with half-cent ties rounded upward, then adds delivery. Delivery itself is not discounted. It excludes taxes, multiple currencies, partial refunds, inventory, payment authorization and monetary formatting. Keeping the task bounded lets you test it completely enough to discuss the reasoning.
Google's code-review guidance examines design, functionality, complexity and tests. Use those questions rather than turning review into a list of stylistic preferences. A change that looks cleaner is not an improvement if it silently changes the rounding contract.
Sources[1]
The apparently simple float calculation returns the wrong cent
The deliberately flawed version multiplies the subtotal by one minus the discount rate, truncates the result with an integer conversion, and adds delivery. For subtotal 1,001 cents, a 10% discount and delivery 499 cents, it returns 1,399 cents.
| Step | Contract result | Flawed calculation |
|---|---|---|
| Subtotal | 1,001 cents | 1,001 cents |
| Discount rate | 1,000 basis points | 1,000 basis points |
| Rounded discount | 100 cents | Not calculated separately |
| Discounted subtotal | 901 cents | Truncated to 900 cents |
| Delivery | 499 cents | 499 cents |
| Total | 1,400 cents | 1,399 cents |
Under the stated contract, the discount is 100.1 cents, rounded to 100. The discounted subtotal is therefore 901 cents and the final total 1,400. The defect is not merely that a float appears in the code: the flawed expression truncates the discounted subtotal instead of rounding the discount according to the requirement.
Preserve this example as a regression. A reviewer should be able to run one test and see the mismatch before reading the patch. Do not “fix” it by changing the expected result to match the implementation.
Implement the chosen rounding rule without binary floating point
For nonnegative inputs, calculate the rounded discount as the integer quotient of subtotal multiplied by discount basis points plus 5,000, divided by 10,000. The added half-denominator implements upward rounding at an exact half-cent under this specific nonnegative rule. Then subtract the discount from subtotal and add delivery.
For a one-cent subtotal and a 50% discount, the raw discount is half a cent and the contract rounds it to one cent. The total before delivery becomes zero. That boundary distinguishes this contract from rounding half to even or truncating the discount. The exercise chooses a rule explicitly rather than claiming one convention is universally correct for commerce.
Validate types before arithmetic. In Python, Boolean is a subclass of integer, so a broad integer-instance check can accept True as one cent. The implementation deliberately checks the exact integer type for these three inputs. Floats, text and out-of-range rates receive clear errors rather than a guessed conversion.
Test examples, identities and invalid inputs
The runnable file includes tests for no discount, a full discount, zero subtotal, zero delivery, the one-cent regression and the half-cent boundary. It rejects negative amounts, rates above 10,000, Boolean inputs and floating-point amounts. Python's unittest module provides the assertions and runner; no external package is needed.
Add structural checks. A full discount should leave exactly the delivery charge. Adding an amount to delivery should increase the total by that same amount. For fixed subtotal and delivery, increasing the discount must not increase the total. The included bounded sweep tests these properties across small subtotals and selected rates.
Property checks complement examples; they do not prove every property for unbounded inputs or every language implementation. Python's integer arithmetic avoids fixed-width overflow here, but a port to another runtime needs its own numeric-range review. Do not assume the same expression behaves identically in every environment.
Sources[2]
Write the review comment as a concrete request
An effective original comment is: “This truncates the discounted subtotal, while the requirement rounds the discount first. With subtotal 1,001, rate 1,000 and delivery 499, the current result is 1,399 but the contract gives 1,400. Please calculate the rounded discount separately and add this regression plus the half-cent case.”
That comment names the behavior, input, expected result and requested change. “Rounding is wrong” leaves the author to guess. “Never use floats” is broader than the actual diagnosis and still would not specify the required rounding convention.
Keep unrelated style changes out of the corrective patch. A small diff and focused tests make review easier and reduce the chance that a functional change is hidden among renaming and formatting. If the requirement is genuinely ambiguous, resolve it before asserting that one implementation is correct.
Deliver the flawed example, correction and executed checks
The download contains the intentionally flawed function, the corrected implementation, explanatory comments and self-contained tests. Running the file executes the checks; the flawed result is asserted to differ in the teaching test rather than leaving a permanently failing test suite. There is no network call, payment action or dependency installation.
Write a brief patch note explaining the contract, the one-cent counterexample, validation behavior and excluded scope. Do not call the result production-ready payment software. A complete checkout system has responsibilities far beyond this arithmetic function.
For an internship application, describe it as an independent code-review exercise. Explain how you moved from a failing case to a specification-preserving patch and which tests prevent recurrence. That is a credible account of engineering work without inventing users, revenue saved or employer approval.