CSC/ECE 517 Fall 2014/oss E1502 wwj

E1502: Questionnaire Controller Refactoring

red writing

Project Description

What it does:

Used on the admin side of Expertiza for creating/ editing questionnaires (rubrics, surveys and quizzes). It helps in add/removing questions, options, etc for a questionnaire.

Very big controller that handles a lot more than the name suggests. Functionalities need to be moved to appropriate controllers.

Quiz methods are should be treated the same as any other type of questionnaire; differences between quiz questionnaires and other questionnaires should be implemented in the model class, quiz_questionnaire

Turn the questionnaire into a “form object.” The ..._questions methods: save_new_questions, delete_questions, save_questions should be in a separate class.

Other classes involved:

questionnaire.rb

quiz_questionnaire.rb

questions_controller.rb

What needs to be done:

Move quiz related functions to quiz_questionnaire.rb.

copy, update_quiz, valid_quiz methods, clone_questionnaire_details is too long.

Debug output (print statements) should be removed.

Understand the functions in the controller and comment them. Ensure that the code is understandable to the next programmer who works on it.

What We Have Done

Method Refactoring

Method Name	Changes Made	Reason For Change
copy	Extracted the content of this method as copy_questionnaires method and put it in questionnaire.rb	The content of this method is about operations on the database (coping a questionnaire), it is better to put it in the model
valid_quiz	Moved this method to quiz_questionnaire.rb	This method is about validation of the quiz, it shouldn't appear in the controller
export	Moved this method to questionnaire.rb	This method exports the questionnaires as csv file, it should't appear in the controller
import	Moved this method to questionnaire.rb	This method imports the questionnaires from csv file, it should't appear in the controller
clone_questionnaire_details	Deleted this method due to the duplication	Substituted by copy_questionnaires method in questionnaire.rb

Format Refactoring

Case 1:
Change from "for question in @questionnaire.questions" to "@questionnaire.questions.each do |question|"
Before

 # Remove a given questionnaire
  def delete
    @questionnaire = Questionnaire.find(params[:id])

    if @questionnaire
      begin
        name = @questionnaire.name
<b>
        for question in @questionnaire.questions</b>
          current_q_type = QuestionType.find_by_question_id(question.id)
          unless current_q_type.nil?
            current_q_type.delete
          end
        end
        @questionnaire.assignments.each{
          | assignment |
          raise "The assignment #{assignment.name} uses this questionnaire. Do you want to <A href='../assignment/delete/#{assignment.id}'>delete</A> the assignment?"
        }
        @questionnaire.destroy
        undo_link("Questionnaire \"#{name}\" has been deleted successfully. ")
      rescue
        flash[:error] = $!
      end
    end

    redirect_to :action => 'list', :controller => 'tree_display'
  end

After

# Remove a given questionnaire
  def delete
    @questionnaire = Questionnaire.find(params[:id])

    if @questionnaire
      begin
        name = @questionnaire.name

        @questionnaire.questions.each do |question|
          current_q_type = QuestionType.find_by_question_id(question.id)
          if current_q_type
            current_q_type.delete
          end
        end
        @questionnaire.assignments.each{
          | assignment |
          raise "The assignment #{assignment.name} uses this questionnaire. Do you want to <A href='../assignment/delete/#{assignment.id}'>delete</A> the assignment?"
        }
        @questionnaire.destroy
        undo_link("Questionnaire \"#{name}\" has been deleted successfully. ")
      rescue
        flash[:error] = $!
      end
    end

    redirect_to :action => 'list', :controller => 'tree_display'
  end