When providing feedback in a code review, the goal is to be clear and constructive without being directive or overly critical. Conventional comments help in this by offering a structured format that's easy to parse and act upon. Let's compare two examples:
Why did you name this variable 'x'? It makes no sense.
This comment is unhelpful for several reasons:
It is confrontational and doesn't provide a clear reason for the issue.
It lacks context or an alternative, leaving the author without a clear direction.
It's more of a criticism than constructive feedback, which can be discouraging.
[Nitpick]: I suggest we rename 'x' to 'transactionCount'. Using more descriptive variable names can improve code readability and maintainability.
This is a well-structured conventional comment:
[Nitpick] immediately informs the author that this is a minor point.
The comment provides a specific suggestion, which is actionable.
It includes a rationale, helping the author understand the value of making this change.
By using conventional comments, the reviewer communicates feedback in a way that is respectful, actionable, and informative. It directs the author on how to improve the code while also providing the reasoning behind the feedback, which is essential for the author's growth and learning.
This function is too complicated.
This type of comment can be frustrating for the author because it:
Lacks specificity about what makes the function complicated.
Doesn't offer any guidance on how to improve it.
Might come across as a blunt criticism, which isn't constructive.
[Issue]: The function `calculateInterest` has multiple nested loops and conditions, which increases its complexity. Consider breaking it down into smaller, more focused functions to enhance readability and make it easier to test.
This comment is clear and helpful:
[Issue] labels the comment as something that needs attention.
It specifies the problem (nested loops and conditions) and offers a clear solution.
The comment is framed in a way that is about the code, not the coder, maintaining a professional and positive tone.
I don't like how this is implemented.
This comment is subjective and unconstructive because:
It's based on personal preference rather than objective standards.
It does not explain why the current implementation is problematic.
It provides no suggestions for improvement.
[Suggestion]: The current implementation of the `fetchData` method might lead to potential race conditions with asynchronous calls. Would it be possible to use async/await here to ensure that the data fetching logic is more predictable and easier to manage?
This improved comment is more effective because:
[Suggestion] indicates that the reviewer is proposing an alternative rather than demanding it.
It explains the potential issue with the current approach and offers a specific, modern alternative.
It invites a discussion, which can lead to a collaborative improvement.
This looks wrong. Are you sure it's supposed to be like this?
This comment can be confusing and unhelpful:
It's vague about what "this" refers to and what exactly looks "wrong".
It questions the author's intention in a way that might come across as condescending.
[Question]: I noticed that the `saveToDatabase` method doesn't handle exceptions. Could you elaborate on the error handling strategy for database operations? It might be beneficial to ensure that any exceptions are caught and logged to avoid silent failures.
This comment provides clear value by:
[Question] indicates that the reviewer is seeking to understand the author's approach.
It points out a specific potential issue (lack of exception handling) and the implications (silent failures).
It suggests a proactive approach (catching and logging exceptions), which contributes to the robustness of the code.