Code Reviewing
In this course, you will read your classmates' code and give them comments about it. This document describes the whys and hows of the 6.005 code reviewing process.
You can't learn how to write without learning how to read, and programming is no exception. Code reviewing is widely used in software development, both in industry and in open source projects. Some companies like Google have instituted review-before-commit as a required policy. You can't get a line of code into the Google source code repository unless another Google engineer has read it, given feedback about it, and signed off on it.
The benefits of code review in practice are several. Reviewing helps find bugs, in a way that's complementary to other techniques (like static checking, testing, assertions, and reasoning). Reviewing uncovers code that is confusing, poorly documented, unsafe, or otherwise not ready for maintenance or future change. Reviewing also spreads knowledge through an organization, allowing developers to learn from each other by explicit feedback and by example. So code reviewing is not only a practically important skill that you will need in the real world, but also a learning opportunity.
Code reviewing is also one way to broaden participation in the course. We will be inviting alums of 6.005 (and its predecessor 6.170) to participate in reviewing code. Many of them are out working in industry or startups, and they have experience in the trenches of software development that they can share.
What to Look For
Although you can make comments about anything you think is relevant, the primary goal of this class is to learn how to write code that is safe from bugs, easy to understand, and ready for change. Read the code with those principles in mind. Here are some concrete examples of problems to look for:
Bugs or potential bugs. Disagreement between code and specification. Off-by-one errors. Risky variable scopes. Optimistic, undefensive programming.
Unclear, messy code. Bad variable or method names. Inconsistent indentation. Convoluted control flow (if and while statements) that could be simplified. Packing too much into one line of code, or too much into one method. Failing to comment obscure code. Having too many trivial comments that are simply redundant with the code.
Misunderstandings of Java. Misuse of == or .equals(). Misuse of arrays or Lists.
Misusing (or failing to use) essential design concepts. Incomplete or incorrect specification for a method or class. Representation exposure for a data abstraction. Immutable datatypes that expose themselves to change. Invariants that aren't really invariant, or aren't even stated. Failure to implement the Object contract correctly (equals and hashCode).
Positive comments are also a good thing. Don't be afraid to make comments about things you really like, for example:
Unusually elegant code
Creative solutions
Great design
Process
We will be using a new web-based code-reviewing system called Caesar, built here at MIT. Caesar is different from other code review tools (like Rietveld and Review Board) in several ways. It's designed for reviewing whole programs, rather than changesets. It divides up a program into small pieces (methods), assigns them to multiple reviewers, and supports discussion-based reviewing with threaded comments and up and down votes. It automatically ignores widely-repeated code, like boilerplate or library code, in order to focus attention on code that's worth the reviewers' time. With this system, you will read small bits of code written by several other students, and many other students will each read a small bit of yours.
Here's how the process will typically go. Only problem set code will be reviewed. Team projects will not go through this process.
Thursday midnight: problem set is due.
Friday morning: submitted code is loaded into Caesar, and an announcement goes out by email to all reviewers that the reviewing process is open.
Friday/Saturday: you should visit Caesar, read the methods that you were assigned, and make comments about them. For each method you review, you can:
-
make a comment
by clicking on a line of code or selecting a range of lines, and typing your comment.
-
reply to another comment
by clicking on Reply. You can use this to clarify, expand on, or disagree with comments made by other reviewers.
-
upvote a comment
by clicking the Thumbs-up icon. Use this to agree with a comment made by another reviewer. Some comments in the system are made automatically by a style checker (Checkstyle), and it's particularly useful for human reviewers to upvote or downvote its comments as appropriate.
-
downvote a comment
by clicking on the Thumbs-down icon. If you're disagreeing with a human, then it's polite and helpful to also Reply to explain why.
Saturday midnight: code reviews are due.
Sunday: course staff (TAs, LAs, lecturer) review the reviews: upvoting, downvoting, replying, and commenting as appropriate, and looking for examples worth highlighting for the whole class.
Monday: Look at your own code! Also, highlights of the code review—great code, problem code, great review comments—are collected and shared with everybody through the Piazza forum and the Caesar dashboard.
Privacy and Visibility
As a code author, you are anonymous. The system does not display your name with any of the code you wrote. Please don't put your full name, username, email address, or other identifying information in your source code. We try to strip out your name and username if it appears, but this process isn't perfect, especially since some of you have names that are pretty common in source code.
As a reviewer, you are fully identified. The system displays your name and username with every review comment, reply, upvote, or downvote that you were responsible for. All registered users can see your name and what you wrote.
All reviewers, all code, and all comments are visible to registered users. The "all users" link shows a list of all users, and each user has a profile page showing all the comments that they have made. A URL for a chunk of code or a review comment can be viewed by anybody logged in to Caesar, regardless of whether they were assigned as a reviewer for that particular chunk.
Caesar is accessible only to registered users. The system is not open to the net, nor is it indexed by Google. It is only visible to members or alumni of 6.005/6.170.
Reputation
As a reviewer in Caesar, you have a reputation score, shown in parentheses after your name. You earn reputation points as follows:
- +1 pt for each upvote (by another user) of a comment you wrote
- +5-10 pts for writing a comment that is selected for highlighting
- +100 pts for passing 6.005/6.170
If you start reviewing early, then you are more likely to get upvotes. If you write good comments, then you are more likely to get upvotes.
As a student in 6.005, your numeric reputation score has no effect whatsoever on your grade. Your participation grade does depend heavily on code reviewing, but the reputation score itself is irrelevant to that. Instead, we will base your participation grade on the quantity and quality of the actual reviewing that you've done, by looking at the activity history on your profile page.
Respect
Be polite. Sarcasm, insults, and belittling words have no place in a code review. It doesn't matter whether you're talking about a person (a fellow reviewer or a code author) or about code. Don't call code "stupid," because that transfers all too easily to the author of the code, whether you meant it that way or not.
Be constructive. Don't just criticize, but be helpful: point the way toward solutions. "Hopeless mess" is not a constructive comment; "name the variables more descriptively, e.g. tmp1 is not a great name" is much more constructive.
As a reviewer, you should downvote comments by others that are unconstructive or rude, and serious problems should be referred to the teaching staff.
As a code author, you should read your feedback carefully, and keep an open mind. Don't get defensive. If your reviewers—who are MIT students and alums—find your code confusing, then you should consider what this says about its clarity and maintainability in the real world. If you disagree with a comment, you can reply to it and engage the reviewer in a discussion, but keep in mind that this makes your identity visible.
FAQ for Reviewers
Does reviewing affect my grade? Yes, it contributes to your participation grade; see the Reputation section above.
Does my reviewing affect other people's grades? Not directly. Problem set grades do not take code reviewing into account, and other reviewers' participation grades are not affected by your upvotes or downvotes. But your reviewing will hopefully help other students become better programmers and acquire a better understanding of the course, and indirectly improve their grades.
How many methods do I need to review, and how much do I need to do on each one? The number you are assigned will depend on the size of the problem set, but will not exceed 10 methods for students (5 for alums). At a minimum, you must do at least one thing on each method assigned to you—make a comment, reply to a comment, upvote, or downvote.
I don't feel like I know anything. How can I possibly review other people's code? You know more than you think you do. You can read for clarity, comment on places where the code is confusing, look for problems we talked about in lecture or recitation, etc.
What if I can't find anything wrong? You can write a positive comment about something good. Or you can simply say "looks good to me", also known as #lgtm.
What are these "checkstyle" comments? They are generated automatically by a style checker (Checkstyle). Some of these comments might be wrong or inappropriate for the situation, so please use upvoting and downvoting to review Checkstyle too.
How much of the whole program do I have to try to understand? Caesar presents each method in isolation, and asks for comments just for that method. We don't expect you to look at anything else, or to spend time struggling to figure out whether it's correct. If you want to see the rest of the program, you can do so using the "view all code" link. But again, it's not expected.
How can I find out who wrote this code? Code authors are anonymous until they reveal themselves—e.g. by replying to a comment and stating that they're the code author. You can write a comment asking a code author to reveal their identity, or contact you, if you wish. If there is a serious situation requiring the identity of the code author—e.g., a potential case of plagiarism—then bring it to the attention of the teaching staff.
FAQ for Code Authors
Can somebody use my code to cheat in the class? Since code reviewing doesn't start until after the deadline, this would be a problem only for students with extensions. Those students may not review code until after their extended deadline.
Is it OK to break my anonymity? Yes. Please let the reviewing process end before you get involved in any discussions of your code, but after that, if you want to join the discussion or directly contact one of your reviewers, you can do so.
Can I look at the reviews of my code while reviewing is still in progress? Yes, you can watch your reviews during the reviewing period, but don't jump in defensively.