Color Mode

Unit 2 Code Review

In this assignment, you will learn to conduct effective code reviews by examining your teammates' Express.js practice websites directly on GitHub. Code review is a fundamental skill in professional development that helps teams maintain quality, catch issues early, and share knowledge.

You will learn what to look for in Express.js code, practice using GitHub's review tools, and provide constructive feedback to help your teammates improve their projects. Since your teammates may be at different stages of MVC refactoring, you'll learn to evaluate code at various levels of organization.

Understanding What to Look For

Before reviewing code, you need to know what constitutes good vs. problematic Express.js code. Here are the key areas and common issues, organized by whether the code has been refactored to MVC architecture or not.

Feature Removal Issues (All Projects Should Address)

Regardless of refactoring progress, all projects should have removed the products feature as part of the MVC assignment. Check for:

❌ Products Feature Not Properly Removed

✅ Products Feature Properly Removed

Pre-Refactoring Code Issues

If your teammate hasn't started MVC refactoring yet, watch for these traditional Express.js issues:

❌ Poor Structure (Monolithic)


        [project-root]
        ├── server.js        # Everything in one file
        ├── style.css        # Should be in public/css/
        ├── home.ejs         # Should be in views/
        └── logo.png         # Should be in public/images/

❌ Common Problems in server.js


        // Unnecessary 'next' parameter
        app.get('/catalog', (req, res, next) => {
            res.render('catalog'); // next is never used
        });

        // Poor variable names
        const x = req.params.cat;
        const y = req.query.sort || 'def';
        
        // Missing validation
        app.get('/catalog/:id', (req, res) => {
            const courseId = req.params.id; // What if this is invalid?
            res.render('course-detail', { id: courseId });
        });

Post-Refactoring Code Issues (MVC Architecture)

If your teammate has started or completed MVC refactoring, look for these organization and architecture issues:

❌ Poor MVC Organization


        [project-root]
        ├── src/
        │   ├── controllers/
        │   │   └── everything.js    # All controllers in one file
        │   ├── models/
        │   │   └── data.js          # Generic naming
        │   └── middleware/
        │       └── stuff.js         # Unclear purpose
        └── server.js

✅ Good MVC Structure


        [project-root]
        ├── src/
        │   ├── controllers/
        │   │   ├── catalog/
        │   │   │   └── catalog.js
        │   │   ├── index.js
        │   │   └── routes.js
        │   ├── models/
        │   │   └── catalog/
        │   │       └── catalog.js
        │   ├── middleware/
        │   │   └── global.js
        │   └── views/
        └── server.js

❌ Controllers with Mixed Concerns


        // Controller doing too much
        export const catalogPage = (req, res) => {
            // Data should be in models, not controllers
            const courses = {
                'CS121': { title: 'Intro Programming' },
                'CS162': { title: 'Intro CS' }
            };
            
            // Business logic should be in models too
            const sortedCourses = Object.values(courses).sort((a, b) => 
                a.title.localeCompare(b.title)
            );
            
            res.render('catalog', { courses: sortedCourses });
        };

✅ Clean Controller Implementation


        import { getAllCourses } from '../../models/catalog/catalog.js';

        export const catalogPage = (req, res) => {
            const courses = getAllCourses(); // Data comes from model
            
            res.render('catalog', {
                title: 'Course Catalog',
                courses: courses
            });
        };

❌ Middleware Split Unnecessarily


        // Multiple middleware functions doing similar things
        app.use(addImportantLocalVariables);
        app.use(addOptionalLocalVariables);
        app.use(addMoreStuff);

✅ Consolidated Middleware


        // Single middleware that handles all res.locals setup
        app.use(addLocalVariables);

Quick Practice

Look at this code and identify issues (this could be found in either a monolithic server.js or a controller file): 


        app.get('/catalog/:cat/:id', (req, res, next) => {
            const c = req.params.cat;
            const i = req.params.id;
            
            if(c==='courses'){
            res.render('course-detail',{category:c,id:i});
            }else{
            res.send('Not found');
            }
        });
Click to see the issues
  • Unused next parameter → Remove from the function header
  • Poor variable names (c, i) → Use full names like category and id
  • Inconsistent formatting → Add blank lines and indentation consistently
  • Using res.send() → Use res.render() and actually render an error page or template
  • No validation for req.params.id → Add validation or fail gracefully

Communication Guidelines

Write issues and feedback as if you're helping a teammate, not grading them. Remember that code at different stages of refactoring requires different types of feedback:

Be Helpful and Adaptive

Keep It Simple

Examples of Good Feedback Tone

Assignment Instructions

1. Get Repository Links

Share your GitHub repository link with your teammates and get at least two repository links to review. You will review these entirely through GitHub's web interface.

2. Review Using GitHub

For each repository, navigate through the files using GitHub's web interface. Since teammates may be at different stages of MVC refactoring, adapt your review approach:

For All Projects (Regardless of Refactoring Status):

If Project Is Not Yet Refactored:

If Project Has Started/Completed MVC Refactoring:

3. Create GitHub Issues

When you find significant problems, create GitHub issues. Here's how:

  1. Navigate to the file with the issue
  2. Click the line number where the problem occurs
  3. Click the three dots (…) that appear
  4. Select "Reference in new issue"
  5. Write a clear title and description

4. Writing Good GitHub Issues

Keep GitHub issues simple and helpful. Here are examples of well-written issues for both types of projects:

Example 1: Products Feature Not Removed


        Products route still exists after refactoring

        Found a products route that should have been removed as part of the MVC assignment cleanup:

        ```javascript
        app.get('/products', (req, res) => {
            // This entire route should be deleted
        });
        ```

        Please remove this route completely, along with:
        - Any products link in your navigation
        - The products.ejs template file if it exists
        - Any products-related variables or data

Example 2: MVC Organization Issue


        Controller contains data that should be in models

        The catalogPage controller is storing course data directly instead of using the model layer:

        ```javascript
        // This data should be moved to src/models/catalog/catalog.js
        const courses = { 'CS121': { title: 'Intro' } };
        ```

        Consider moving this data to your catalog model and importing it:
        ```javascript
        import { getAllCourses } from '../../models/catalog/catalog.js';
        const courses = getAllCourses();
        ```

Example 3: Traditional Code Quality Issue


        Unused next parameter in catalog route

        The catalog route includes a `next` parameter but never uses it:

        ```javascript
        // Change this:
        app.get('/catalog', (req, res, next) => {
        
        // To this:
        app.get('/catalog', (req, res) => {
        ```

        This follows Express best practices and makes the code cleaner.

Example 4: Import Path Issue (MVC Projects)


        Incorrect import path in routes.js

        The import statement for the catalog controller appears to have an incorrect path:

        ```javascript
        // Check if this path is correct
        import { catalogPage } from './catalog.js';
        ```

        Based on your directory structure, it should probably be:
        ```javascript
        import { catalogPage } from './catalog/catalog.js';
        ```

Create 2-3 meaningful issues per repository you review. Focus on issues that would actually improve the code, whether they are related to MVC organization or general code quality.

Take the time to do this right

Code review is not just about finding problems — it is about understanding how code works, learning from different approaches, and helping your teammates write better software. Each issue you create should teach something valuable, either to the person receiving the feedback or to you as the reviewer. This process sharpens your ability to read code critically, recognize patterns that lead to maintainable applications, and communicate technical concepts clearly. Remember that in professional development, the quality of your code reviews directly impacts team productivity and code quality, so developing this skill now will benefit your entire career.

Review Checklist

Since you may have limited experience performing code reviews, use this comprehensive checklist as a guide to conducting a thorough review. This detailed guidance will help you identify issues you might otherwise miss and develop the systematic approach that professional developers use. Only check on items that are relevant to your teammate's current refactoring progress:

Feature Cleanup (All Projects)

Traditional Express.js Code (If Not Yet Refactored)

MVC Architecture (If Refactoring Started/Completed)

Code Quality (All Projects)

EJS Templates (All Projects)

Assignment Submission Template

Fill out the form below and click the download button at the bottom. A new page will open and automatically prompt you to print as a PDF. Submit that PDF to your instructor. Take care to provide professional, well-written responses that clearly explain the technical concepts. This assignment demonstrates your ability to articulate technical concepts clearly in written form:

Unit 2 Code Review
Repositories Reviewed
Refactoring Status:
GitHub Issues Created:
Reflection
Write 2-3 paragraphs, not bullet points, reflecting on:
  • What you learned about code quality from reviewing others' work
  • How reviewing both traditional and MVC-structured code affected your understanding
  • What you discovered about the benefits and challenges of code organization
  • How this experience will help you write better code yourself
  • How you approached giving constructive feedback to teammates at different stages of refactoring

Keep in mind that your ability to write clearly and communicate professionally will be evaluated in this assignment. It's important to express your ideas in an articulate and comprehensive manner.

Completion Confirmation
I have notified both teammates that their code reviews are complete and they can view the GitHub issues I created. I notified them via on .

Final Steps

  1. Complete your reviews and create GitHub issues as described above
  2. Create your submission document using the form above as a guideline
  3. Submit the PDF to Canvas
  4. Notify your teammates that you have finished reviewing their code and they can check their GitHub issues

Key Concepts Summary

This assignment taught you to systematically evaluate Express.js code at different stages of organization, use GitHub's collaboration tools, and provide constructive technical feedback. You learned to adapt your review approach based on whether code follows traditional monolithic structure or modern MVC architecture.

The experience of reviewing code at different refactoring stages helps you understand the evolution of code organization and the benefits that proper architecture provides. These skills are essential in professional development where code review helps maintain quality and share knowledge among team members.

You also practiced identifying when features should be removed during refactoring, which is an important skill in maintaining clean, purposeful codebases as requirements evolve.