Code Review 1

In the first round of code reviews, each team will present a portion of its design in class and other students will comment on the design and offer suggestions for improvement. Each team will also receive written code reviews on their project, including partial reviews from other students in the class and a full review from me. After the discussions in class, I will meet individually with each team to go over my code review in detail. The rest of this document describes how we will conduct the code reviews in class.

Presenters

To start off each code review, the presenting team will spend 10-15 minutes to address the following issues:

For each of the classes under discussion, describe the key idea(s) behind that class. In particular, describe the information and/or design decisions that are hidden within that class.
If you considered alternate designs, say what the alternatives were and how you chose between them.
Go over (briefly) a few key methods, discussing the API for each briefly, in order to give the audience a feel for what are the pieces within the class.
Walk through an example of usage, such as deleting a range of text or the life cycle of an undoable operation.
Talk briefly about what you found hard and easy in designing your system.
What are elements of your design that you are happiest about? What are its weaknesses? Ideally, you will already be aware of all of the issues discovered by reviewers.

You should prepare slides for your presentation. Remember the overall goal of abstraction: finding a simple way to think about something that is internally complicated. Try to find a simple way to explain your design, so that even people who have not read the code can easily get the basic idea; think about what are the most important elements of your design. Of course, if there are tricky elements, you will also need to mention those as well (don't pretend something is simple when it really isn't).

Reviewers

After the team has presented its code, several other students will offer their comments. Reviewers must read over the relevant code before class and prepare a written code review on cs190codereview.appspot.com. You can also download the project code from GitHub if you'd like to use your favorite IDE to read through it (all of the projects will be readable to everyone in the class). You don't need to read the entire project (though you are welcome to if you wish); you only need to read the parts related to the area being reviewed. Enter comments in the code review tool, but do not publish your comments yet. The comments will be saved and you will be able to see them, but no one else will see them until you publish them. After we have discussed the relevant code in class, then you should publish your comments. The "right" number of comments to enter is probably in the range of 10-20, depending on the complexity of the code you are reviewing and the number of useful issues you can identify (but don't invent issues if you can't find 10 things to comment on).

Here are some general things to think about as you review other people's code. See the specific review topics below for additional criteria to consider.

How easy was it for you to find and understand the relevant code? Are the interfaces and algorithms clear? Does the documentation provide all the information you would need if you were going to make modifications to the existing code? Ideally there should be nothing that you find confusing. It may be easier for you to understand this code because you have already implemented something similar, so try to put yourself in the position of someone who doesn't already have experience with this particular application.
How effective is the class decomposition? Do the classes have simple interfaces that hide information effectively? Can you think of alternate class decompositions that would be better (e.g. are the classes too thin? too thick?)
Is the code simple and obvious? Identify the most complicated parts.
Any of the other topics we have discussed in class are also fair game.

You will have about 5 minutes to present your code review in class. In addition to addressing the general issues above, your review should include two specific things:

Pick a particularly egregious example of bad documentation (actual documentation that is not great, as opposed to missing documentation). Show the code and the existing documentation, then rewrite the documentation to improve it.
Identify a specific example of information leakage (knowledge that is unnecessarily distributed in several places, which creates code dependencies). Try to explain this in a way that everyone in the class can understand (show a slide with the code, if that helps). Then describe how the code could be modified to eliminate that information leakage (e.g. introducing a new class, or changing existing APIs).

Overall, say what you liked best about the design, and what you think is the greatest potential area for improvement. You will not have enough time in class to present all of the comments from your online code review, so focus on the things that are most important.

Review Topics

Each code review will focus on one of the following aspects of the editor design. If you are a presenter or reviewer for this topic, please address the issues described below.

Text storage

Presenters: how do you represent the textual data being edited, and what APIs or classes manage this representation? How (and where) is the text accessed, both for making modifications (such as deleting the selected text) and for displaying it on the screen? How does code refer to particular positions within the text?

Reviewers: to what degree is the management of text encapsulated in a single place, vs. sprinkled across the system? Are there multiple copies of portions of the text? If the representation of text were changed (e.g. to handle files so large that the entire file can't be kept in memory at once, or to support rich text with additional formatting), how many places in the code would have to be modified?

Undo

Presenters: how is information for undoing and redoing represented? What are the APIs, both for recording information and for invoking the undo/redo operations? How do you implement undo grouping?

Reviewers: to what degree is the implementation of undo/redo encapsulated, vs. sprinkled across the system? Suppose you wanted to undo additional things, such as the scroll position or the size of the window: how many places in the code would have to change?

Schedule

Date	Topic	Presenters	Reviewers
Friday, April 22	Text storage	Penkov, Starosta	Gadangi, Y. Li, Walker
		Broder, Rodriguez	Pradhan, Charitsis, S. Li
Monday, April 25	Text storage	Walker	S. Li, Starosta, Broder, Pradhan
		Charitsis, Y. Li	Penkov, Man, Rodriguez, Gadangi
Wednesday, April 27	Undo	Li, Man	Rodriguez, Walker, Starosta, Y. Li
		Gadangi, Pradhan	Man, Broder, Penkov, Charitsis

Reviewers will present in the order listed above. Everyone should bring their laptop to class for the code reviews, both for your use in presenting, and also so that you can browse the code online while we are discussing it.