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 as an official part of the class, but if you would like to discuss my code review, send me an email and we can schedule a time early next quarter.

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. What knowledge and/or design decisions are hidden within this class? Does knowledge from other classes leak into this one?
  • Show a summary of the external interface of the class, such as method signatures and any exported structs.
  • If it helps to understand the class, walk through an example of usage.
  • If you considered alternate designs, say what the alternatives were and how you chose between them.
  • What were the biggest challenges you faced in designing these classes?
  • 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. Please pay special attention to modularity: the degree to which information and knowledge are encapsulated in once place, so that other places do not have to be aware of them.

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).

In this round of reviews, please pay special attention to modularity and information hiding:

  • Does the project find elements that can be cleanly separated out into different classes (or methods within a class)?
  • Is there effective information hiding, with each piece of knowledge encapsulated in one module so that the rest of the program need not be aware of it?
  • Conversely, is there information leakage, where knowledge that could be encapsulated leaks out to other modules? For example, how many classes have some awareness of dependencies?
  • Consider how much each module needs to know about the rest of the world, versus the functionality that it provides. The best modules are those that provide a lot of functionality without having to know much about the rest of the world. This is somewhat analogous to depth, but instead of considering what the module exposes to the rest of the world (its interface), consider what the module must know about the rest of the world.
  • Does each module have a clear abstraction (it has a small set of closely related functions), or is it more of a random assortment of features?

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 for each project.

Kathy/Shreya (make6)

Review the Target class and the stringutil namespace. As part of the review, consider how well line numbers are encapsulated: how much code has to be aware of them?

Cooper/George (make2)

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

Chendi/Chloe (make8)

Review the Builder class plus util.h and util.cpp. As part of the review, consider the degree to which variable expansion is encapsulated.

Luca/Will (make4)

Review the ExecutionEngine and Scheduler classes.

Alex/Briana (make7)

Review the Rule, Recipe, and TargetBuilder classes.

Jerry/Kevin (make3)

Review the mechanisms for managing dependencies.

Schedule

Presenters (Repo)Review&PresentReview
Kathy/Shreya (make6)Anna, GeorgeAkram, Alex
Cooper/George (make2)Akram, WillMichelina, Shashank
Chendi/Chloe (make8)Elliot, MichelinaCooper, Kevin
Luca/Will (make4)Briana, JerryManya, Chloe
Alex/Briana (make7)Shashank, ChendiKathy, Luca
Jerry/Kevin (make3)Manya, ShreyaAnna, Elliot

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.