Code Review 3

This round of code reviews will proceed much like the reviews for Project 2. For each review, the project developers will present a portion of their design in class and other students will comment on the design and offer suggestions for improvement. Each presenting team will also receive written code reviews from other students in the class. All teams will receive a written code review from me. I will not be meeting with teams individually for this project.

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 the life cycle of parsing a Makefile line.
• Talk briefly about what you found hard and easy in designing your system.
• What are the 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 is abstraction: finding simple ways to think about things that are 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

As in the past, each project will be reviewed by four students from other projects, according to the table below. Reviewers must read over the relevant code before the given class and prepare a written code review on GitHub. To do this, go to the project in GitHub, click on the "Pull requests" tab, click on the "Project3" pull request, and click on the "Files changed" tab. Click on a line on the right side to enter comments for that line, then select "Start a review". You can 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 on GitHub, but do not submit your review yet. The comments will be saved and you will be able to see them, but no one else will see them until you submit the review. After we have discussed the relevant code in class, then you should submit your review. I suggest providing 10-20 comments, 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 meaningful things to comment on).

As you did in earlier reviews, look for red flags in the code you review, and also consider the following overall questions:

• Is the class decomposition appropriate? One possible error is to lump too many things together in one class, where it could have been possible to separate out some pieces cleanly. Another possible error is to over-decompose, resulting in shallow classes, dependencies, and information leakage.
• Are APIs general-purpose or specialized?
• Is the code obvious (everything immediately makes sense, your first guesses about behavior turn out to be correct, and it's easy to see that the code is complete and correct)? When aren't obvious, think about why that is the case, and how they could be made more obvious.

Review Presentations

Two of the reviewers for each project will present their code reviews in class, after the team has introduced its design. You will have 5 minutes for your presentation. This will not be enough time for you to discuss your review in detail, so pick out the most interesting and important issues. Use slides to structure your presentation and display examples. Start by saying what you liked best about the design, and what you think is the biggest opportunity for improvement. Then describe a few of the most important red flags that you found; be specific, and show examples. If you identified solutions to problems, pick the one that is most interesting and tell us about it.

Review Topics

Here are the specific topics we will be reviewing in this round for each project.

Keegan/Sanjay (make6)

Review the PrerequisiteManager and ShellCommandExecutor classes.

Sam/Michaela (make5)

Review the TaskGraph, TaskNode, and TinyMakeNode classes, along with the mechanism for running tasks (such as the thread pool).

Kathleen/Gokul (make2)

Review the mechanism for managing line numbers. How do line numbers find their way from the Makefile to error messages, such as those that occur for Makefile syntax errors or when executing commands? Is the line number mechanism cleanly encapsulated? How did the need for line numbers impact the rest of the design? For this review, it isn't necessary to review entire classes; instead, you may end up reviewing parts of several classes to follow the management of line numbers.

Charles/Dean (make1)

Review the mechanism for managing line numbers. How do line numbers find their way from the Makefile to error messages, such as those that occur for Makefile syntax errors or when executing commands? Is the line number mechanism cleanly encapsulated? How did the need for line numbers impact the rest of the design? For this review, it isn't necessary to review entire classes; instead, you may end up reviewing parts of several classes to follow the management of line numbers.

Rohan/Colin (make7)

Review the mechanism for parsing Makefiles, including how information from the parse is represented (the parser, tiny_make, and utils classes).

Landis/Luis (make8)

Review the mechanism for parsing Makefiles, including how information from the parse is represented.

Schedule