Refactor on_the_fly_calc #2039
Refactor on_the_fly_calc #2039
Conversation
Generated by expertiza-bot |
1 similar comment
|
Tested on virtualbox - no errors that I could find when editing assignments, creating new ones, or viewing them as a student. Expected, since the only real change was the module names and file locations. When methods from the new assignment_score_helper module are called, they are still called on assignment instances. Assignment simply includes the entire module and its methods. Low-risk refactor |
app/helpers/assignment_helper.rb
Outdated
| end | ||
|
|
||
| def compute_reviews_hash | ||
| @review_scores = {} |
There was a problem hiding this comment.
why are these being passed as instance variables, can you refactor all of this into using parameters
app/helpers/assignment_helper.rb
Outdated
| contributors.each do |contributor| | ||
| questions = peer_review_questions_for_team(contributor, round) | ||
| assessments = ReviewResponseMap.assessments_for(contributor) | ||
| assessments = assessments.select {|assessment| assessment.round == round } |
There was a problem hiding this comment.
use select!{ |assessment| assessment.round == round}
app/helpers/assignment_helper.rb
Outdated
| # Get all of the questions asked during peer review for the given team's work | ||
| def peer_review_questions_for_team(team, round_number = nil) | ||
| topic_id = SignedUpTeam.find_by(team_id: team.id).topic_id unless team.nil? or SignedUpTeam.find_by(team_id: team.id).nil? | ||
| review_questionnaire_id = review_questionnaire_id(round_number, topic_id) unless team.nil? or SignedUpTeam.find_by(team_id: team.id).nil? |
There was a problem hiding this comment.
use || instead of or
app/helpers/assignment_helper.rb
Outdated
|
|
||
| # Get all of the questions asked during peer review for the given team's work | ||
| def peer_review_questions_for_team(team, round_number = nil) | ||
| topic_id = SignedUpTeam.find_by(team_id: team.id).topic_id unless team.nil? or SignedUpTeam.find_by(team_id: team.id).nil? |
There was a problem hiding this comment.
these are literally the same checks so just clean this up
|
Hey @nnhimes, |
Refactoring on_the_fly_calc.rb to be in assignment_helper.rb
The file has 7 methods: 3 that are exclusively called as assignment instance methods, and 4 that are called internally to help facilitate those 3. The name on_the_fly_calc no longer made sense, and it belongs in helpers