tes2
Table of contents
Code Review Responses: Add Context When It Counts
When responding to code review comments, responses like âDone,â âUpdated,â or âFixedâ are commonly used to indicate addressing a suggestion. However, sometimes, a little extra context adds a lot of clarity.
Next time you resolve a code review comment, ask yourself: "Is how I addressed the comment completely obvious from the code change and comment thread?" If not, supplement your response with a brief note to clarify the âwhyâ or âhow.â Your reviewers will thank you.
When is it helpful to add context to a code review comment response? Here are a few examples:
Your code change doesn't fully explain how you addressed the comment. Providing a brief summary helps the reviewer verify the changes without re-examining every line of the delta, and creates a clearer historical record.
Reviewer:
This approach seems risky. It might not handle all the edge cases properly.
Less helpful response:
More helpful response:
Author:
Updated.
Good catch. I've added checks for null, empty, and negative inputs, each with a new test case. Thanks!
You made a design choice or trade-off that isn't self-evident. Capturing the reasoning behind a choice provides valuable context. Note that non-obvious design choices within the code should ideally be explained in code comments or the commit description as well.
Reviewer:
Consider using a more performant library for this data transformation.
Less helpful response:
More helpful response:
Author:
Iâll go with Y.
Done. I considered Library X, but stuck with Library Y because our datasets here are typically small, so the performance difference is negligible, and Library Y has a much simpler API.
An offline discussion influenced the solution. Briefly summarizing the outcome or key reasoning from an offline sync ensures that other reviewers, who only see the final code change, can grasp the âwhyâ.
Reviewer:
This logic seems a bit complex. Consider a simpler way to handle these.
Less helpful response:
More helpful response:
Author:
Fixed.
As we discussed offline, this complexity is required to maintain backward compatibility with legacy data formats. Iâve added a comment in the code to clarify this. Thanks!
There are multiple ways to address the comment. Clearly stating which option you selected and the reasoning behind that choice over other alternatives helps reviewers.
Learn more code review practices in Googleâs code review guide: google.github.io/eng-practices/review.
Construct with Collaborators, Call with Work
Classes require various objects and parameters to function. The "Construct with Collaborators, Call with Work" guideline can help you construct effective inputs:
Use the constructor for collaboratorsâthe dependencies that establish the objectâs identity. Collaborators stay with the object for its lifetime to enable it to fulfill its ongoing duties.
Pass workâthe parameters that change with each interactionâto methods. Unique to each call, these inputs provide the specific data needed for an operation such as a file path or database query.
Consider a ReportGenerator that needs a database, a formatter, and a date range to generate a report. The database and formatter, as collaborators, are injected via the constructor, while dateRange, which varies per report generation, is passed as a method parameter to the generate method:
class ReportGenerator {
private final Database database;
private final Formatter formatter;
// database and formatter are passed as collaborators.
ReportGenerator(Database database, Formatter formatter) {
this.database = database;
this.formatter = formatter;
}
// dateRange is passed as a parameter.
Report generate(Range<Instant> dateRange) {
return formatter.format(database.getRecords(dateRange));
}
}
A single ReportGenerator object can generate multiple reports with different date ranges:
ReportGenerator generator = new ReportGenerator(database, formatter);
Report report1 = generator.generate(dateRange1);
Report report2 = generator.generate(dateRange2);
Following the "Construct with Collaborators, Call with Work" guideline promotes:
Reusability: Enables instances to be used for multiple, distinct operations.
Testability: Separates dependency setup from business logic.
Cleaner code: Hides implementation dependencies from the objectâs users.
Predictable behavior: Locks in dependencies at creation time.
Note that the definition of "collaborator" versus "work" depends on the object's identity. For example, a RequestMessage could be a collaborator for a RequestHandler if the handler operates on a single request, or work if the handler processes different requests with each method call.
Recommended articles
Comments
Be the first to comment.