CSC/ECE 517 Spring 2023 - E2325 Further refactoring and improvement of review mapping helper

Link to our PR

https://github.com/expertiza/expertiza/pull/2577

Introduction and Prior Work

Previously the file review_mapping_helper.rb was refactored according to the requirements mentioned by code climate. On analysis, it was found that the module exceeded the lines of code and several methods in the file had high cyclomatic complexity and assignment branch condition size.

All the issues detected by the code climate analysis were fixed by refactoring the code. All methods were tested for cyclomatic complexity, perceived complexity and assignment branch condition size. Rubocop was used for individually testing all methods and screenshots for successful results were included in the documentation.

Requirements

1. The method get_data_for_review_reports passes a data structure to views/reports/_review_report.html.erb making it responsible for handling data. This violates the Expert pattern which needs to be rectified.

2. Methods involving the term "color" are not clear. Refactor them to make them more understandable.

3. Convert method names to more Ruby-like method names.

4. Use the standardized score calculation code from response_map.rb to compute awarded review score.

5. Generalize the method sort_reviewer_by_review_volume_desc to sort by any metric and separate the logic for number of review rounds.

6. Separate chart-generating methods into a separate file.

7. Refactor method list_review_submission.

8. There is less clarity on some of the methods created, add comments, refactored and made them easier to comprehend.

Design Plan

Implementing Expert Pattern

As mentioned in the requirements, the method get_data_for_review_reports marshals a lot of data together and passes back a data structure. It is used in views/reports/_review_report.html.erb, but it is quite difficult to see how the data is being displayed. It violates the Expert pattern, because the view needs to know how to break apart the structure that is passed to it. We plan on making use of partials to make the code easier to follow

Use updated code

There has been an introduction of logic for score calculation. We need to make use of this standardized logic for score calculation in the method get_awarded_review_score.

We followed the instructions of referring the given repository. From pull requests we referred (https://github.com/expertiza/expertiza/pull/2038/files#diff-d11f696c186b0d742896eb3b6e7a89a8f8af62af817040da242a3887c936f461) for code but, the code was missing in the repository hence we were unable to refactor it.

Improve method names

Few method names like get_team_color, get_data_for_review_report, get_awarded_review_score need to be changed to more ruby like method names. For e.g. the method name get_data_for_review_report can be changed to review_report_data. This name follows the ruby naming conventions and is concise and clear to understand. This refactoring will be done for all the methods in all the files where these methods are called.

Renaming 1

Renaming 2

Renaming 3

Renaming 4

Renaming 5

Renaming 6

Extract chart generation

There are several methods used for chart generation in review reports.

The existing logic was contained in the file app/helpers/review_mapping_helper.rb.

The task was to keep them in a separate file. So, all the methods were removed and inserted in a new file app/helpers/review_chart_helper.rb

By extracting and later inserting in a different file, this created several other issues like more LOC, these issues are also handled in this project.

These are the code snippets of the changes made -

Here the display_volume_metric_chart function was broken down into smaller functions to reduce it's complexity. Each reduced function performs only one operation.

First the display_volume_metric_chart function was broken down into two main functions: 1)build_chart_data 2)build_chart_options

To improve the efficiency, the build chart data function is further broken down into build_chart_dataset function.

The build_chart_dataset function comprises of build_chart_y-axis which plots and assigns various attributes like thickness and percenage to the y-axis

Generalize Logic

We were instructed to modify the method sort_reviewer_by_review_volume_desc to generalize it so that it can sort by any metric, not just review volume. It should be able to sort based upon other metrics like 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. For this we can update the sorting logic and take in the parameter which upon which the sorting needs to be done.

To solve this issue, upon analyzing the methods and functions written we came to the conclusion that for different metrics, different functions have to be written. Example - currently it is calculating using volume, so there is a function call which calculates volumes and sorts them in descending order, so similarly for different metrics, different functions have to be written

Refactoring and Lucidizing code

We plan on refactoring the required methods and add comments, refactor code to make it easier to comprehend and follow.

Refactored Code

• This is the refactored version of the method which was broken into simpler functions to reduce assignment branch condition size.

Here it is seen that it returns the response maps data structure to the view. This needs to be handled and a partial will be used to work with the data. This will be in accordance with the expert pattern.

• The logic from response.rb is to be used for score calculation

• Apart from above implementations, other requirements will be performed by refactoring code including restructuring, renaming and simplifying logic to make it comprehensible. Seperate files will be added wherever needed, method names will be changed to follow ruby naming conventions and methods with no comments will be refactored to add comments that will explain their functionality.

Files Included

The main file that we will work on is review_mapping helper. For some of the requirements, like implementing expert pattern, we will have to make changes in the views/reports/_review_report.html.erb file.

Apart from these files, while refactoring method names, all the files in which the methods are called will be impacted.

Test Plan

Testing with RSpec

Majorly we will not be adding new methods for this implementation. We plan on not breaking the test cases that were previously written. For the last project all 61 test cases were successfully passing. Apart from this we will perform manual testing to verify the UI functionality for reviews.

Rubocop Testing

We performed rubocop testing on all the methods included in review_mapping_helper.rb to check for cyclomatic complexity, perceived complexity and assignment branch condition size which can be seen here. The results for all these tested methods are included in the documentation for our last project.

1)def display_volume_metric_chart

2)def display_tagging_interval_chart(intervals)

3)def sort_reviewer_by_review_volume_desc

4)def initialize_chart_elements(reviewer)

5)def calculate_key_chart_information(intervals)

6)def review_metrics(round, team_id)

7)def get_data_for_review_report(reviewed_object_id, reviewer_id, type)

8)def get_team_color(response_map)

9)def submitted_within_round?(round, response_map, assignment_created, assignment_due_dates)

10) def get_data_for_review_report(reviewed_object_id, reviewer_id, type)

11) def get_awarded_review_score(reviewer_id, team_id)

12)def list_review_submissions(participant_id, reviewee_team_id, response_map_id)

13) def check_submission_state(response_map, assignment_created, assignment_due_dates, round, color)

Team Members

1. Dakshil Kanakia (drkanaki)

2. Kunal Shah (kshah24)

3. Ritwik Vaidya (rvaidya2)

Mentor: Jialin Cui