CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb

This wiki page is for the description of changes made under E1608 OSS assignment for Spring 2016, CSC/ECE 517. The assignment task is to refactor & write unit tests for [question_type].rb (eg. criterion.rb).

Problem Statement

Files related

Four models is related: criterion.rb, scale.rb, checkbox.rb, questionnaire_header.rb, upload_file.rb. There are 4 methods “edit”, “view_question_text”, “complete” and “view_completed_question” correspond to different views.

edit ~> when instructor add/edit new questions
view ~> when instructor view existed questions
complete ~> when students do peer reviews
view_completed_question ~> when student view existed peer reviews

Expertiza question types inheritance hierarchy

Choice question
- Scored question
  - Scale
  - Criterion
- Unscored question
  - Dropdown
  - CheckBox
TextResponse
- TextArea
- TextField
UploadFile

Files problem

There are some issues in each file, such as security, complexity, duplication, etc. And currently there is no unit tests for [question_type].rb.

Our contribution

Refactor these [question_type].rb files and fix issues.
Keep the inputs and outputs of these methods (edit, view, complete, view_completed_question) the same as before.
Write unit tests for [question_type].rb listed above.
Create RSpec files for each question type in /spec/models/ folder
Create multiple tests to check valid and invalid cases, such as input including special character double quote (“”).

Refactor

Criterion.rb

Method has too many lines: Line 35 - 158 [106/30]

Wrap the specific code into different instance methods.

Cyclomatic complexity for complete is too high: Line 35 - 158 [28/6]

Wrap the code into different instance methods to reduce cyclomatic complexity

def complete(count, answer=nil, questionnaire_min, questionnaire_max, dropdown_or_scale)

   if self.size.nil?
     cols = '70'
     rows = '1'
   else
     cols = self.size.split(',')[0]
     rows = self.size.split(',')[1]
   end

html = '

<label for="responses_' +count.to_s+ '">' +self.txt+ '</label>'
   #show advice for each criterion question
   question_advices = QuestionAdvice.where(question_id: self.id).sort_by { |advice| advice.id }
   advice_total_length = 0

   question_advices.each do |question_advice|
     if question_advice.advice && !question_advice.advice == ""
       advice_total_length += question_advice.advice.length
     end
   end

   if question_advices.length > 0 and advice_total_length > 0
     # html += '<a id="showAdivce_' + self.id.to_s + '" onclick="showAdvice(' + self.id.to_s + ')">Show advice</a>'
     # html += '<script>'
     # html += 'function showAdvice(i){'
     # html += 'var element = document.getElementById("showAdivce_" + i.toString());'
     # html += 'var show = element.innerHTML == "Hide advice";'
     # html += 'if (show){'
     # html += 'element.innerHTML="Show advice";'
     # html += '}else{'
     # html += 'element.innerHTML="Hide advice";}'
     # html += 'toggleAdvice(i);}'
     #
     # html += 'function toggleAdvice(i) {'
     # html += 'var elem = document.getElementById(i.toString() + "_myDiv");'
     # html += 'if (elem.style.display == "none") {'
     # html += 'elem.style.display = "";'
     # html += '} else {'
     # html += 'elem.style.display = "none";}}'
     # html += '</script>'
     #

      # html += ''
     # code added to reduce complexity
      html = complete_show_advice(self, html)

     #[2015-10-26] Zhewei:
     #best to order advices high to low, e.g., 5 to 1
     #each level used to be a link;
     #clicking on the link caused the dropbox to be filled in with the corresponding number
     question_advices.reverse.each_with_index do |question_advice, index|
       html += '<a id="changeScore_>' + self.id.to_s + '" onclick="changeScore(' + count.to_s + ',' + index.to_s + ')">'
       html += (question_advices.length - index).to_s + ' - ' + question_advice.advice + '</a>
'
       html += '<script>'
       html += 'function changeScore(i, j) {'
       html += 'var elem = jQuery("#responses_" + i.toString() + "_score");'
       html += 'var opts = elem.children("option").length;'
       html += 'elem.val((opts - j - 1).toString());}'
       html += '</script>'
     end

      html += ''
   end

   if dropdown_or_scale == 'dropdown'

      # html += '<select id="responses_' +count.to_s+ '_score" name="responses[' +count.to_s+ '][score]">'
     # html += '<option value=>--</option>'
     # for j in questionnaire_min..questionnaire_max
     #   if !answer.nil? and j == answer.answer
     #     html += '<option value=' + j.to_s + ' selected="selected">'
     #   else
     #     html += '<option value=' + j.to_s + '>'
     #   end
     #   if j == questionnaire_min
     #     html += j.to_s
     #     html += "-" + self.min_label if self.min_label && self.min_label.length>0
     #     html += "</option>"
     #   elsif j == questionnaire_max
     #     html += j.to_s
     #     html += "-" + self.max_label if self.max_label && self.max_label.length>0
     #     html += "</option>"
     #   else
     #     html += j.to_s + "</option>"
     #   end
     # end

      # html += "</select>"
     # html += '<textarea cols=' +cols+ ' rows=' +rows+ ' id="responses_' +count.to_s+ '_comments" name="responses[' +count.to_s+ '][comment]" style="overflow:hidden;">'
     # html += answer.comments if !answer.nil?

      # html += '</textarea>

'

      ## code added  to reduce Cyclomatic Complexity
      html = complete_drop_down(self, html, count, answer, questionnaire_min, questionnaire_max)

    elsif dropdown_or_scale == 'scale'
      # html += '<input id="responses_' +count.to_s+ '_score" name="responses[' +count.to_s+ '][score]" type="hidden"'
      # html += 'value="'+answer.answer.to_s+'"' if !answer.nil?
      # html += '>'
      #
      # html += ''
      # html += ''
     # for j in questionnaire_min..questionnaire_max

      #   html += ''
     # end

      # html += ''
     #
     # # if !self.min_label.nil?

      # #   html += ''
     # # else

      # #   html += ''
     # # end
     # ## code added  to remove duplicated code
     #  html = complete_min_label_condition(self, html)
     #
     #
     # ##
     # # for j in questionnaire_min..questionnaire_max

      # #   html += ''
      # # end
      # ## code added  to remove duplicated code
      #  html = complete_questionnaire_min_to_questionnaire_max(self, html, answer, questionnaire_min, questionnaire_max)
      #
      # html += '<script>jQuery("input[name=Radio_' +self.id.to_s+ ']:radio").change(function() {'
      # html += 'var response_score = jQuery("#responses_' +count.to_s+ '_score");'
      # html += 'var checked_value = jQuery("input[name=Radio_' +self.id.to_s+ ']:checked").val();'
      # html += 'response_score.val(checked_value);});</script>'
      #
      # # if !self.max_label.nil?
      # #   html += ''
     # # else

      # #   html += ''
     # # end
     # ## code added  to remove duplicated code
     #  html = complete_max_label_condition(self, html)
     #

      # html += ' <label>' +j.to_s+ '</label> 
' +self.min_label+ ' <input type="radio" id="' +j.to_s+ '" value="' +j.to_s+ '" name="Radio_' +self.id.to_s+ '"'
     # #   html += 'checked="checked"' if (!answer.nil? and answer.answer == j) or (answer.nil? and questionnaire_min == j)

      # #   html += '> ' +self.max_label+ ' '
     # html += '<textarea cols=' +cols+ ' rows=' +rows+ ' id="responses_' +count.to_s+ '_comments" name="responses[' +count.to_s+ '][comment]" style="overflow:hidden;">'
     # html += answer.comments if !answer.nil?
     # html += '</textarea>

'

     ## code added  to reduce Cyclomatic Complexity
      html = complete_scale(self, html, count, answer, questionnaire_min, questionnaire_max)
   end
   html.html_safe
 end

Identical code found in 1 other location: Line 20 - 32 (mass = 89)
Identical code found in 1 other location: Line 135 - 139 (mass = 50)
Identical code found in 1 other location Line 13 - 14 (mass = 40)
Similar code found in 1 other location Line 103 - 105 (mass = 26)
Similar code found in 2 other locations Line 152 - 153 (mass = 22)
Similar code found in 3 other locations: Line 130 - 134 (mass = 18)

Scale.rb

Cyclomatic complexity for complete is too high. Method has too many lines
Identical code found in 1 other location: line 17-28
Identical code found in 1 other location: line 50-54
Identical code found in 1 other location: line 10-11
Similar code found in 3 other locations: line 60-64

CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb

Contents

Problem Statement

Files related

Expertiza question types inheritance hierarchy

Files problem

Our contribution

Refactor

Criterion.rb

Scale.rb

Checkbox.rb

Questionnaire_header.rb

Upload_file.rb

Unit-test

Criterion.rb

Scale.rb

Checkbox.rb

Upload_file.rb

Navigation menu

	<label>' +j.to_s+ '</label>
' +self.min_label+ '		<input type="radio" id="' +j.to_s+ '" value="' +j.to_s+ '" name="Radio_' +self.id.to_s+ '"' # # html += 'checked="checked"' if (!answer.nil? and answer.answer == j) or (answer.nil? and questionnaire_min == j) # # html += '>	' +self.max_label+ '

CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb

Problem Statement

Files related

Expertiza question types inheritance hierarchy

Files problem

Our contribution

Refactor

Criterion.rb

Scale.rb

Checkbox.rb

Questionnaire_header.rb

Upload_file.rb

Unit-test

Criterion.rb

Scale.rb

Checkbox.rb

Upload_file.rb

Navigation menu

Search