CSC/ECE 517 Spring 2024 - E2406 Refactor review mapping helper.rb

Team

Mentor

• Ananya Mantravadi (amantra)

Team Members

• Ravi Ghevariya (rghevar)
• Manan Patel (mrpatel8)
• Kanishk Harde (knharde)

Relevant Links

• Link to Expertiza Repository: https://github.com/expertiza/expertiza
  

• Link to Forked Repository: https://github.com/kanishkharde/expertiza
  

• Link to Pull Request: https://github.com/expertiza/expertiza/pull/2802
  

Expertiza Background

Expertiza is a Ruby on Rails framework-based open-source web application. It is kept up by NC State University employees and students. With the help of this program, instructors can fully manage the assignments and responsibilities assigned in their classes. Expertiza provides many strong features, such as subject addition, group creation, and peer review administration. It is a flexible platform that can manage many kinds of tasks. Users can consult the Expertiza wiki to get more in-depth details about the many capabilities that Expertiza offers.

About Helper

The Ruby on Rails review_mapping_helper module offers a number of helper methods to make the peer review process easier in assignments. It has the capacity to calculate review scores, manage submission statuses, generate review reports, and visualize review metrics; however, it needs to be refactored in order to make the code more readable and maintainable.

Problem Statement

The existing codebase derived from E2301 suffers from lack of clarity, violating the Expert pattern, and inefficient methods, hindering code understandability and maintainability. Key issues include convoluted data handling in views, unclear color-coding methods, and inefficient sorting functions. Additionally, redundant methods and unclear feedback_response_maps further obscure the code's purpose.

Tasks

1. Refactor get_data_for_review_report to use partials for clearer code in views/reports.
2. Refactor color-related methods for clarity and consistency, using American spelling.
3. Rename methods starting with "get" to adhere to Ruby conventions.
4. Update get_awarded_review_score to utilize standardized score-calculation code.
5. Generalize sort_reviewer_by_review_volume_desc for sorting by various metrics.
6. Extracting chart-generating methods into their own module or evaluating the necessity of list_review_submissions
7. Review and clarify the purpose of feedback_response_maps methods.
8. Add comments or refactor small classes for clarity and documentation.

Phase 1

We have concentrated our efforts on addressing the following difficulties throughout the project's first phase:
1. Refactor get_data_for_review_report to use partials for clearer code in views/reports.
2. Refactor color-related methods for clarity and consistency, using American spelling.
3. Rename methods starting with "get" to adhere to Ruby conventions.
8. Add comments or refactor small classes for clarity and documentation.

Phase 2

We have planned our efforts on addressing the following difficulties throughout the project's second phase:
4. Update get_awarded_review_score to utilize standardized score-calculation code.
5. Generalize sort_reviewer_by_review_volume_desc for sorting by various metrics.
6. Extracting chart-generating methods into their own module or evaluating the necessity of list_review_submissions
7. Review and clarify the purpose of feedback_response_maps methods.

Implementation

Phase 1

Refactor the `get_data_for_review_report` method

Refactor the `color-related` methods

Rename methods starting with `get` to adhere to Ruby conventions methods

Design Pattern

The Refactoring pattern is essential to enhance code readability, maintainability, and adherence to best practices. By systematically restructuring code components, eliminating redundancies, and applying standard conventions, the Refactoring pattern ensures improved code quality and easier future modifications.

Plan of work

We began our work by examining the review_mapping_helper.rb file to understand its connection to the rest of the codebase. We then had detailed discussions with our mentor about the project's flow and structure. We organized our project requirements by difficulty levels, assigned tasks among the team, and began refactoring existing methods and creating new files as needed. We made sure to preserve the integrity of existing test cases while adding new ones where needed.

Project 4 - DESIGN DOC

We have been assigned below-cited changes to do after project 3

4. Refactoring get_awarded_review_score Method:

• Standardize the score-calculation logic to ensure consistency and readability.
  
• Break down complex calculations into smaller, more understandable components.
  
• Improve method documentation and clarity to enhance understanding.
  

5. Generalizing sort_reviewer_by_review_volume_desc:

• Modify the method to accept flexible sorting metrics, enabling customization based on various criteria.
  
• Update method documentation to reflect the changes and provide usage guidelines.
  

6. Code Organization Enhancements:

• Extract chart-generating methods into separate modules or classes for better organization and reusability.
  
• Evaluate the necessity of list_review_submissions method based on insights from the Expertiza wiki and refactor accordingly.
  

7. Documentation and Clarity Improvements:

• Review methods related to feedback_response_maps and add comments or refactor small classes to enhance clarity and documentation.
  
• Ensure consistent naming conventions and adherence to coding standards throughout the codebase.
  
  

This section contains changes and contributions to Project 4

---

Issue #4: Method get_awarded_review_score needs to be refactored

• Methods involved: get_awarded_review_score
  
• Solution:After addressing Issue 2, the method was renamed to determine_awarded_review_score to align with Ruby conventions. Additionally, redundant instance variable settings were moved inside the loop to streamline the code. The explicit nil or -1.0 checks were eliminated and integrated into the instance_variable_set line.
  
• Changes

---

Issue #5: The method sort_reviewer_by_review_volume_desc should be generalized so that it can sort by any metric, not just review volume. Other metrics might include number of suggestions, or number of suggestions + number of problems detected. This method should not be counting the number of review rounds! Since other places in the code will need to know the number of review rounds, it should be calculated somewhere else in the system.

• Methods involved: sort_reviewer_by_review_volume_desc
  
• Problem: The current function, sort_reviewer_by_review_volume_desc, arranges reviewers in descending order based solely on review volume. This limited approach means it can't be applied to sorting by other metrics.
  
• Solution:The idea is to create a more versatile approach by changing the name of sort_reviewer_by_review_volume_desc to sort_reviewer_desc and allowing a metric argument for sorting reviewers, as demonstrated below. This modification was also applied to the review_report section where it was originally used.
  
• Changes

---

Issue #6: make a separate file for for chart related elements in review_mapping_helper.rb

• Methods involved: display_volume_metric_chart(), display_tagging_interval_chart()
  
• Previous Code: The methods mentioned used to be included in the review_mapping_helper.rb file, but they should have been in their own helper file.
  
• Problem: These techniques carry out comparable functions in creating graphs and can be implemented as either distinct modules or mixins. Additionally, each method contains more than 30 lines, making them excessively lengthy.
  
• Solution: Created new module named 'Charts' withinthe same file. This way the chart helper methods are separated from review_mapping_helper where they are out of place and difficult for any developer to find.
  
• Changes

---

Issue #7: Several methods for feedback_response_maps, check docs to manage this confusion

• Methods involved: feedback_response_for_author()
  
• Previous Code: previous code had an ambiguous variable name
  
• Problem: The purpose of the variable was unclear.
  
• Solution: Rename the variable to enhance its clarity and provide comments to explain its purpose clearly.
  
• Changes

---

Test Plan

This project involves refactoring, meaning that the original functionality should remain unchanged. Tasks included adding comments to existing code, altering variable names, and similar adjustments that didn't affect the test cases. When method names were modified or moved to different files, corresponding changes were made in the relevant RSpec files. All test cases passed successfully, indicating that the refactoring didn't break the existing functionality.

The image below shows the test cases are passing as well as most of the code climate issues were resolved.