CSC/ECE 517 Spring 2022 - E2239: Further refactoring and improvement of review mapping helper.rb

From Expertiza_Wiki
Jump to navigation Jump to search

Background

This project builds on the work done by E2125, which focused on simplifying the methods in review_mapping_helper. This project focuses on refactoring that seeks to make the current code more understandable and transparent.

Problems and planned changes

Problem: The method get_data_for_review_report violates the Expert pattern

get_data_for_review_report passes back a data structure with multiple response objects to the _review_report.html.erb view. This violates the Expert pattern, because the given view does not know how to break apart the structure passed to it.

  • Proposed Solution : get_data_for_review_report will be refactored to use partials, which


Problem: Several method names begin with get

This naming convention is not Ruby-like, and needs to be changed

  • Proposed Solution : All method names beginning with get will be refactored to use more appropriate names


Problem: The method get_awarded_review_score needs to be updated

get_awarded_review_score computes an overall score based upon scores awarded in individual rounds. This is one of many places in Expertiza where scores are being calculated. Score-calculation code for multiple rounds is being standardized now, in response_map.rb .

  • Proposed Solution : Change the method to utilize the standard code from response_map.rb


Problem: The method sort_reviewer_by_review_volume_desc needs to be generalized

sort_reviewer_by_review_volume_desc only sorts by review volume, which is very specific.

  • Proposed Solution : Modify the method to be able to sort by multiple different metrics such as number of suggestions, or number of suggestions + number of problems detected. This will also jusity a method name change (and possible parameter change/additions), which will have to be updated elsewhere in the app.


Problem: The method feedback_response_maps violates the Expert pattern

It is not clear why feedback_response_maps are here.

  • Proposed Solution :

Look up the expertiza wiki for the documentation, and make it clearer what they are doing


Problem: three small classes violates the Expert pattern

It is not clear why ReviewStrategy , StudentReviewStrategy , TeamReviewStrategy are here.

  • Proposed Solution :

Look up the expertiza wiki for the documentation, add comments, and Refactor three classes


GitHub links and Pull Request

GitHub link here

Team Members

Evan Brown

Chaitanya Patel

Rachel Hyo Son

Andrew Pippin