CSC/ECE 517 Fall 2024 - E2451. Reimplement feedback response map.rb
This page contains the details of OSS project E2451: Reimplement feedback response map done for Expertiza in Fall 2024.
Expertiza and Project Overview
Background
Expertiza is an open-source application used in CSC 517 (Object Oriented Design and Development) as a platform for creating teams, assigning programs, and reviewing submissions, among other tasks. Expertiza is also used to help students develop a working application in order to mimic how students will work on programs in the workplace.
Project Overview
The goal of this project is to refactor the feedback_response_map.rb file from the Expertiza repository and integrate it into the reimplementation-back-end repository. Within Expertiza, when a user submits a review, an instance of the Response class is created, associated with a ResponseMap. Each ResponseMap links a reviewer (reviewer_id), reviewee (reviewee_id), and the reviewed item (reviewed_object_id). FeedbackResponseMap, a subclass of ResponseMap, represents the feedback provided by a reviewee on the feedback received for their submission.
This class has significant room for improvement, especially with respect to object-oriented principles. Refactoring feedback_response_map.rb will streamline functionality, enhance maintainability, and improve adherence to coding standards.
Current Implementation
The current feedback_response_map.rb in the Expertiza repository has several issues. It includes the following problems:
- Poor adherence to SOLID and DRY principles: The current code structure introduces unnecessary repetition and complexity.
- feedback_response_report Method Issues: This critical method is highly complex, relying on custom data structures and separate containers for each review round, resulting in a DRY violation.
- Improper Responsibility Assignment: The email method does not belong within this class and would benefit from refactoring out into a separate class using the Visitor pattern. This pattern is suggested to streamline functionality, though alternative design patterns may be considered if more suitable.
- Memory Inefficiencies: The method currently creates unnecessary memory through inefficient structures, including multiple separate arrays for different rounds of review, which could be simplified into a 2D array structure.
- Code Smells: The code has redundant methods, unclear variable names, inefficient loops, and overly long method names.
Objectives & Requirements
1. Refactor the feedback_response_report Method
- Address the DRY violation by consolidating the containers for review rounds.
- Generalize the code to handle both cases—when rubrics vary by round and when they do not—without duplicating code.
- Introduce efficient data structures, like a 2D array, to replace multiple arrays and reduce memory usage.
2. Separate Responsibilities Using Visitor Pattern (stretch goal)
- Extract the email method using the Visitor pattern to modularize email functionality. Alternatively, select a design pattern better suited to organizing this method if available. Note that the email method is not properly used in its current location and is better suited to the Response class rather than ResponseMap.
3. Eliminate Redundant Methods
- Remove methods that do not serve a clear purpose and improve method naming for clarity.
- Aim to address any redundant meta-review functionality, as indicated by the professor's preference to remove these from response_map.rb.
4. Refactor for Code Clarity and Efficiency
- Improve variable names, reduce unnecessary loops, and replace long method names with concise, intuitive alternatives.
- Ensure that each method has a single, clearly defined purpose, improving code readability.
5. Testing and Documentation
- Write comprehensive tests for all newly implemented and modified methods to ensure reliability.
- Add detailed comments to facilitate code maintenance and understanding for future developers.
Design
Design Principles
- Single Responsibility Principle (SRP): Each method and class should have a single, well-defined purpose.
- Don’t Repeat Yourself (DRY): Eliminate redundancy by creating general solutions that handle multiple scenarios.
- Ruby Readability Guidelines (Week 9 Notes):
- Guideline: Variable names should be neither too short nor too long.
- Guideline: Names should not be redundant.
- Guideline: A method should do only one thing and do it well.
- Visitor Pattern: Utilize this pattern to separate emailing functionalities from the main class.
Implementation Plan
The refactoring will proceed as follows:
1. Analyze and Refactor Current Code Structure
- Review and understand the existing structure and dependencies within feedback_response_map.rb.
- Identify areas of redundancy, complexity, and inefficiency, particularly in feedback_response_report, to create a clear plan for refactoring.
- Try to reduce character count and improve readability with aliasing (this way we do not break any code that relies on being able to call specific methods).
2. Refactor the feedback_response_report Method
- Consolidate containers for each review round to address DRY violations. By using a single structure to store review data, we will eliminate hard-coded repetition.
- Generalize logic to accommodate both scenarios where rubrics vary by round and where they do not, without duplicating code paths.
- Replace inefficient structures, such as multiple arrays, with more flexible and memory-efficient options, like a 2D array or a hash, to reduce memory usage and improve scalability.
3. Introduce Helper Methods for Reusable Logic (for feedback_response_report)
- Extract frequently used or complex code blocks into helper methods, reducing the size of primary methods and enhancing reusability.
- Use helper methods for specific actions, like handling rounds or retrieving assignment data, to make the code cleaner and better organized.
- Emphasis on adhering to the guideline: A method should do only one thing and do it well.
4. Research Future Refactoring for email Method
- The email method is convoluted and the functionality should be refactored into a different class in the future.
- This is because this visitor pattern violation is present in many classes, and is beyond the scope of a one-class re-implementation.
5. Improve Variable Naming and Structure
- Replace ambiguous variable names with clearer alternatives that convey purpose, such as renaming @temp_review_responses and @temp_response_map_ids to more meaningful names.
- Simplify long method names to make the code more readable and approachable for new contributors.
Because the main focus will be on feedback_response_map.rb, we can take a closer look on what we will do to implement these (point #3 above). Before refactoring, the FeedbackResponseMap class contains 6 methods, with the majority of the code in self.feedback_response_report:
We aim to change that by splitting up this method into multiple helper methods, as seen in the following:
To go into more detail, here is the code for the feedback_response_report method, and how we intend to split it up:
The red section of code will be self.get_feedback_authors, yellow will be self.varying_rubrics_report, and green will be self.static_rubrics_report. Each separated group will be its own method, and self.varying_rubrics_report and self.static_rubrics_report (the yellow and green methods) will be returned at the end of the new feedback_response_report.
Testing
1. Adapt Existing Tests for Refactored Methods
- Any new helper methods will be private, and so by the "Magic Tricks of Testing" guide, should not be tested (both query and command messages sent to self are ignored in the test plan).
- Use mocks to isolate components during testing, especially for external dependencies like Mailer, to ensure that email functionality works as expected without direct integration.
2. Document Code for Clarity
- Write detailed comments explaining the purpose of each method, input and output, and any critical logic that might be complex for future developers to understand.
- Create or update documentation and diagrams to illustrate the newly designed structure, making it easier for new developers to contribute or modify functionality.
Documentation
Ensure all code is well-documented, with comments explaining purpose, inputs, outputs, and any complex logic. Maintain updated documentation for all modified and newly added classes and methods, allowing for ease of understanding and maintenance.
Conclusion
By refactoring feedback_response_map.rb, this project will enhance Expertiza’s backend maintainability and readability, ensuring a modular, efficient, and more secure implementation.
Team Information
Mentor
- Chaitanya Srusti (crsrusti@ncsu.edu)
Members
- Sarah Daniel (sdaniel8@ncsu.edu)
- William Grochocinski (wagrocho@ncsu.edu)
- Tipton Middleton (tgmiddl2@ncsu.edu)
- ChatGPT (Honorary Member)