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

• Github Repository: https://github.com/kanishkharde/expertiza
• Pull Request: https://github.com/expertiza/expertiza/pull/2776

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.
  
  

• We're going to make confusing variables easier to understand by rearranging them.
  

• Even though we tried hard in project 3, we couldn't cover all the code. In project 4, we're going to write more tests and use the test skeleton better.

This section contains changes and contributions to Project 4

---

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

Methods involved: initialize_chart_elements(), display_volume_metric_chart(), display_tagging_interval_chart(), calculate_key_chart_information()

Previous Code: All these methods were previously a part of the review_mapping_helper.rb when it should have its separate helper file
Problem: These methods perform similar tasks of generating graphs and can have there separate module or a mixin and also each of the methods more than 30 lines inside them making them too large
Solution: distributed this code among 2 newly created files named review_mapping_charts_helper.rb and data_mapping_helper.rb within helper folder. 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. review_mapping_charts_helper.rb contains the original methods and data_mapping_helper.rb manages the data need by these chart methods

review_mapping_charts_helper.rb

module  ReviewMappingChartsHelper
...
    def initialize_chart_elements()
    def display_volume_metric_chart()
    def display_tagging_interval_chart()
    def calculate_key_chart_information()

all link related to these changes:
review_mapping_charts_helper.rb
review_mapping_helper.rb
data_mapping_helper.rb
Contributor: Ravi Ghevariya

---

---

• 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: It wasn't clear what the variable was used for.
Solution: refactor the variable name to be more easily understood and add comments to give a clear explanation for why it exists

Contributor: Manan Patel

---

• 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: Currently the method sort_reviewer_by_review_volume_desc sorts the reviewers in descending order but only considers one metric which is review volume which makes it very specific and it cannot be used if we want to sort by other metrics
Solution:The solution here is make this a generalized method by renaming sort_reviewer_by_review_volume_desc to sort_reviewer_desc and accepting a metric as argument on which we can sort the the reviewers as shown below. Since it was used in review_report partial the implementation was changed there as well

review_mapping_helper.rb


app/views/reports/_review_report.html.erb



review_mapping_helper_spec.rb



All link related to these changes:

review_mapping_helper.rb
_review_report.html.erb
review_mapping_helper_spec.rb

Contributor: Kanishk Harde

---

• Issue #9: Method get_awarded_review_score needs to be refactored

Methods involved: get_awarded_review_score
Solution: Post tackling of Issue 2, the name of the method was changed to compute_awarded_review_score to make it more ruby like. Further on, the code which involved redundant setting of instance variables was placed inside the loop to make it more concise. The explicit check for nil or -1.0 was removed and added to the instance_variable_set line.

Before

def compute_awarded_review_score(reviewer_id, team_id)
    # Storing redundantly computed value in num_rounds variable
    num_rounds = @assignment.num_review_rounds
    # Setting values of instance variables
    (1..num_rounds).each { |round| instance_variable_set('@score_awarded_round_' + round.to_s, '-----') }
    # Iterating through list
    (1..num_rounds).each do |round|
      # Changing values of instance variable based on below condition
      if @review_scores[reviewer_id] && @review_scores[reviewer_id][round] && @review_scores[reviewer_id][round][team_id] && @review_scores[reviewer_id][round][team_id] != -1.0
        instance_variable_set('@score_awarded_round_' + round.to_s, @review_scores[reviewer_id][round][team_id].to_s + '%')
      end
    end
  end

After

def compute_awarded_review_score(reviewer_id, team_id)
    num_rounds = @assignment.num_review_rounds

    (1..num_rounds).each do |round|
      score = @review_scores && @review_scores[reviewer_id] && @review_scores[reviewer_id][round] && @review_scores[reviewer_id][round][team_id]
      instance_variable_set("@score_awarded_round_#{round}", "#{score}%") unless score.nil? || score == -1.0
    end
end

All links related to these changes:

review_mapping_helper.rb

Contributor: Aditya Joshi

---

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.

Github 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/?