Code Review 3
This round of code reviews will be structured like previous code
reviews:
- For each project that we review, the developing team will spend
10-15 minutes describing the key design ideas behind a portion of
the project (see below for specific topics for each review).
- Other students will create written code reviews for the topics
specified for the project. You only need to read code related to the
particular topics for the review.
As usual, don't post your reviews until after the
discussion in class.
- Two of the reviewers will spend 3-5 minutes each to present the
most important aspects of their review in class.
- I will write code reviews for all of the projects (I won't
have individual meetings with the teams to discuss the reviews, but if
you have any questions you'd like to discuss, send me an email and we
can schedule a time).
General Issues
Finding a clean decomposition for this project is challenging. When you are
reviewing a project, please consider the
following overall issues in addition to the specific topics listed
for the project below:
- Separation of concerns: to what degree does the project disentangle
the various bits of functionality
of the shell? Ideally there will be at least a few different classes,
and the responsibilities of each class will be clear and distinct
from those of other classes.
Look for places where different tasks are intertwined (multiple problems
all addressed in the same place), or where there is information leakage
(a logical task, such
as handling variables or parsing backslash sequences, has code spread
around in multiple places).
- General-purpose interfaces: ideally, interfaces will be at least a bit
general-purpose; look for those that seem unnecessarily specialized.
- Special cases: does the code have a lot of special cases, which
make it hard to visualize the overall behavior and convince yourself that
the code works? It's a red flag if the
same special-case checks appear in multiple places.
Schedule
Jayden/Blake (clash5) | Akshay, Yulian | None |
Yipeng/Wantong (clash9) | Julia | None |
Jestin/Marc (clash8) | Adrien | None |
Review Topics
Here are the particular topic(s) to review for each project, along with
a few suggested questions to consider. In addition to these questions,
feel free to discuss other design issues, either good or bad,
that you notice in reviewing the code.
Jayden/Blake (clash5) and Yipeng/Wantong (clash9)
Review the overall decomposition of functionality between the various
.cc files. Does
this provide a clean separation of concerns? Are the APIs deep?
Is there information leakage? Are there other things
that could possibly have been separated into different classes?
Jestin/Marc (clash8)
Review the structure of the parser (clash_parser). Is it easy to understand
the structure of the parser and convince yourself that it handles all of
the required constructs? Is the handling of each construct concentrated in
a single place where it's easy to understand, or spread over several places?