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

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
No edit summary
Line 43: Line 43:
* Cyclomatic complexity for complete is too high: Line 35 - 158 [28/6]
* Cyclomatic complexity for complete is too high: Line 35 - 158 [28/6]
Wrap the code into different instance methods to reduce cyclomatic complexity
Wrap the code into different instance methods to reduce cyclomatic complexity
<code>
" <code>
def complete(count, answer=nil, questionnaire_min, questionnaire_max, dropdown_or_scale)
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 = '<li><div><label for="responses_' +count.to_s+ '">' +self.txt+ '</label></div>' #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 += '<div id="' + self.id.to_s + '_myDiv" style="display: none;">' # 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><br/>' 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 += '</div>' end if dropdown_or_scale == 'dropdown' # html += '<div><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></div>" # 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></td></br><br/>' ## 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 += '<table>' # html += '<tr><td width="10%"></td>' # for j in questionnaire_min..questionnaire_max # html += '<td width="10%"><label>' +j.to_s+ '</label></td>' # end # html += '<td width="10%"></td></tr><tr>' # # # if !self.min_label.nil? # # html += '<td width="10%">' +self.min_label+ '</td>' # # else # # html += '<td width="10%"></td>' # # end # ## code added to remove duplicated code # html = complete_min_label_condition(self, html) # # # ## # # for j in questionnaire_min..questionnaire_max # # html += '<td width="10%"><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 += '></td>' # # 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 += '<td width="10%">' +self.max_label+ '</td>' # # else # # html += '<td width="10%"></td>' # # end # ## code added to remove duplicated code # html = complete_max_label_condition(self, html) # # html += '<td width="10%"></td></tr></table>' # 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><br/><br/>' ## code added to reduce Cyclomatic Complexity html = complete_scale(self, html, count, answer, questionnaire_min, questionnaire_max) end html.html_safe end
    if self.size.nil?
</code>"
      cols = '70'
      rows = '1'
    else
      cols = self.size.split(',')[0]
      rows = self.size.split(',')[1]
    end
 
    html = '<li><div><label for="responses_' +count.to_s+ '">' +self.txt+ '</label></div>'
    #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 += '<div id="' + self.id.to_s + '_myDiv" style="display: none;">'
 
      # 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><br/>'
        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 += '</div>'
    end
 
    if dropdown_or_scale == 'dropdown'
      # html += '<div><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></div>"
      # 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></td></br><br/>'
 
      ## 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 += '<table>'
      # html += '<tr><td width="10%"></td>'
      # for j in questionnaire_min..questionnaire_max
      #   html += '<td width="10%"><label>' +j.to_s+ '</label></td>'
      # end
      # html += '<td width="10%"></td></tr><tr>'
      #
      # # if !self.min_label.nil?
      # #   html += '<td width="10%">' +self.min_label+ '</td>'
      # # else
      # #   html += '<td width="10%"></td>'
      # # end
      # ## code added to remove duplicated code
      # html = complete_min_label_condition(self, html)
      #
      #
      # ##
      # # for j in questionnaire_min..questionnaire_max
      # #   html += '<td width="10%"><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 += '></td>'
      # # 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 += '<td width="10%">' +self.max_label+ '</td>'
      # # else
      # #   html += '<td width="10%"></td>'
      # # end
      # ## code added to remove duplicated code
      # html = complete_max_label_condition(self, html)
      #
      # html += '<td width="10%"></td></tr></table>'
      # 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><br/><br/>'
 
      ## code added to reduce Cyclomatic Complexity
      html = complete_scale(self, html, count, answer, questionnaire_min, questionnaire_max)
    end
    html.html_safe
  end
</code>
* Identical code found in 1 other location: Line 20 - 32 (mass = 89)
* 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 135 - 139 (mass = 50)

Revision as of 21:32, 21 March 2016

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 += '' 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

    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

    • Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Spring_2016/Refactor_and_write_unit_tests_for_question_type.rb&oldid=101001"