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? (or variables? or line numbers?)
  • 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.

Joshua/Zhenbang (make3)

Review how Makefiles are parsed and the dependency graph is built. This includes all code executed up until main invokes ExecuteRules, including the files main.cpp, parser.cpp, rule-filter.cpp, and rule-dep.cpp (and their associated header files).

Clinton/Jessica (make6)

Review the management of the dependency graph. This includes the Target class plus any other information about dependencies (prerequisites) stored anywhere else. How is dependency information created during parsing? How (and where) is it stored? How is it used during the "execution" phase of the build?

Nirvik/Yash (make8)

Review how variables are managed, including how they are parsed, how (and where) their values are stored, how subsitutions occur, and how line numbers find their way into error messages associated with variables.

Christina/Sumer (make2)

Review how variables are managed, including how they are parsed, how (and where) their values are stored, how subsitutions occur, and how line numbers find their way into error messages associated with variables.

Krain/Lauren (make4)

Review how Makefiles are parsed. Also review the data structures generated by the parser that are used later on during the "execution" phase of the build. You do not need to review the details of the execution phase.

Ava/Gordon (make1)

Review all of the code that executes after the Makefile has been parsed (i.e. starting at line 227 in Maker.cc). This includes the MakeScheduler and Target classes plus any other classes they invoke, such as ShellExecutor.

Schedule

Presenters (Repo)Review&PresentReview
Joshua/Zhenbang (make3)Lauren, ChristinaAnsh, Yasmine
Clinton/Jessica (make6)Ava, VirginiaThejas, Sumer
Nirvik/Yash (make8)Alex, NathanKrain, Gordon
Christina/Sumer (make2)Yasmine, ZhenbangAlex, Nirvik
Krain/Lauren (make4)Ansh, ThejasJoshua, Clinton
Ava/Gordon (make1)Jessica, YashNathan, Virginia

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.