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

From Expertiza_Wiki
Revision as of 21:42, 21 March 2016 by Yjin6 (talk | contribs)
Jump to navigation Jump to search

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 = complete_show_advice(self, html)
       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'
         ## code added  to reduce Cyclomatic Complexity
         html = complete_drop_down(self, html, count, answer, questionnaire_min, questionnaire_max)
       elsif dropdown_or_scale == 'scale'
          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

    Checkbox.rb

    • Method has too many lines.
    • Cyclomatic complexity for complete is too high.
    • Similar codes in edit method are found in 2 other locations.
    • Similar codes in view_question_text method are found in 4 other locations.

    Questionnaire_header.rb

    Upload_file.rb


    Unit-test

    Criterion.rb

    Scale.rb

    Checkbox.rb

    Upload_file.rb