User:Ptrived: Difference between revisions
Line 57: | Line 57: | ||
@response.email(); | @response.email(); | ||
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :error_msg => error_msg, :save_options => params[:save_options] | redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :error_msg => error_msg, :save_options => params[:save_options] | ||
end | |||
</pre> | |||
====Removing unused variables==== | |||
There were some unused variables in this controller. The edit method contained a variable "array_not_empty" variable which was not used anywhere. So we removed the unused variables. | |||
====Fixing code duplication==== | |||
There was some duplicate code in the update method. So we removed the duplicate code and retained the code snippet inside the if block. Below is the modified code for the update method: | |||
<pre> | |||
#Update the response and answers when student "edit" existing response | |||
def update | |||
return unless action_allowed? | |||
# the response to be updated | |||
@response = Response.find(params[:id]) | |||
msg = "" | |||
begin | |||
@map = @response.map | |||
@response.update_attribute('additional_comment', params[:review][:comments]) | |||
if @map.type=="ReviewResponseMap" && @response.round | |||
@questionnaire = @map.questionnaire(@response.round) | |||
elsif @map.type=="ReviewResponseMap" | |||
@questionnaire = @map.questionnaire(nil) | |||
else | |||
@questionnaire = @map.questionnaire | |||
end | |||
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq } | |||
questions=sort_questions(@questionnaire.questions) | |||
create_answers(params,questions) | |||
questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq } | |||
if !params[:responses].nil? # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric) | |||
params[:responses].each_pair do |k, v| | |||
score = Answer.where(response_id: @response.id, question_id: questions[k.to_i].id).first | |||
unless score | |||
score = Answer.create(:response_id => @response.id, :question_id => questions[k.to_i].id, :answer => v[:score], :comments => v[:comment]) | |||
end | |||
score.update_attribute('answer', v[:score]) | |||
score.update_attribute('comments', v[:comment]) | |||
end | |||
end | |||
if (params['isSubmit'] && (params['isSubmit'].eql?'Yes')) | |||
# Update the submission flag. | |||
@response.update_attribute('is_submitted',true) | |||
else | |||
@response.update_attribute('is_submitted',false) | |||
end | |||
rescue | |||
msg = "Your response was not saved. Cause:189 #{$!}" | |||
end | |||
redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :save_options => params[:save_options] | |||
end | end | ||
</pre> | </pre> |
Revision as of 16:58, 23 March 2016
E1614. Refactoring Response Controller
This page provide the details of the changes done as part of the refactoring response controller project(E1614).
A brief overview of Expertiza
Expertiza project is a platform to create reusable learning objects through peer review. It is an open source project which uses Ruby on Rails framework.
Project Statement
The main aim of this project was to refactor the response_controller. The following tasks were completed as part of refactoring in this project:
- Moving variable declaration to right places.
- Removing unused variables.
- Fixing code duplication.
- Replacing if else block with switch statements.
- Remove the unreachable code.
About Response Controller
The Response controller is responsible for the CRUD(Create, Read, Update and Delete) operations on responses. The users can fill out a questionnaire such as review rubric or feedback on the partner's contribution. So the ResponseController handles the various operations on the responses, which are objects that Expertiza creates when you fill out a questionnaire.
Changes done as part of refactoring
Moving variable declaration to right places
There were two variables "msg" and "error_msg" in the create method which were declared and initialized to blank strings and then again assigned some string value before it was used in the method. So we moved the variable declaration to the place were it was used and removed the unnecessary variable declaration. Below if the code snippet for the changes done:
def create @map = ResponseMap.find(params[:id]) #assignment/review/metareview id is in params id set_all_responses #to save the response for ReviewResponseMap, a questionnaire_id is wrapped in the params if params[:review][:questionnaire_id] @questionnaire = Questionnaire.find(params[:review][:questionnaire_id]) @round = params[:review][:round] else @round=nil end # create the response if params[:isSubmit].eql?('Yes') is_submitted = true else is_submitted = false end @response = Response.create(:map_id => @map.id, :additional_comment => params[:review][:comments],:round => @round, :is_submitted => is_submitted)#,:version_num=>@version) #Change the order for displaying questions for editing response views. questions=sort_questions(@questionnaire.questions) if params[:responses] create_answers(params, questions) end #@map.save msg = "Your response was successfully saved." error_msg="Error" @response.email(); redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :error_msg => error_msg, :save_options => params[:save_options] end
Removing unused variables
There were some unused variables in this controller. The edit method contained a variable "array_not_empty" variable which was not used anywhere. So we removed the unused variables.
Fixing code duplication
There was some duplicate code in the update method. So we removed the duplicate code and retained the code snippet inside the if block. Below is the modified code for the update method:
#Update the response and answers when student "edit" existing response def update return unless action_allowed? # the response to be updated @response = Response.find(params[:id]) msg = "" begin @map = @response.map @response.update_attribute('additional_comment', params[:review][:comments]) if @map.type=="ReviewResponseMap" && @response.round @questionnaire = @map.questionnaire(@response.round) elsif @map.type=="ReviewResponseMap" @questionnaire = @map.questionnaire(nil) else @questionnaire = @map.questionnaire end questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq } questions=sort_questions(@questionnaire.questions) create_answers(params,questions) questions = @questionnaire.questions.sort { |a,b| a.seq <=> b.seq } if !params[:responses].nil? # for some rubrics, there might be no questions but only file submission (Dr. Ayala's rubric) params[:responses].each_pair do |k, v| score = Answer.where(response_id: @response.id, question_id: questions[k.to_i].id).first unless score score = Answer.create(:response_id => @response.id, :question_id => questions[k.to_i].id, :answer => v[:score], :comments => v[:comment]) end score.update_attribute('answer', v[:score]) score.update_attribute('comments', v[:comment]) end end if (params['isSubmit'] && (params['isSubmit'].eql?'Yes')) # Update the submission flag. @response.update_attribute('is_submitted',true) else @response.update_attribute('is_submitted',false) end rescue msg = "Your response was not saved. Cause:189 #{$!}" end redirect_to :controller => 'response', :action => 'saving', :id => @map.map_id, :return => params[:return], :msg => msg, :save_options => params[:save_options] end