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

E1608. Refactor & write unit tests for [question_type].rb (eg. criterion.rb) Contact: Zhewei Hu (zhu6@ncsu.edu) What it does: In last summer we refactored different class of questions to make them follow the same design (you can take a look on Criterion.rb): there are 4 methods “edit”, “view_question_text”, “complete” and “view_completed_question” correspond to different views. Files involved: criterion.rb, scale.rb, checkbox.rb, questionnaire_header.rb, upload_file.rb 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 What’s wrong with it: You can click these [question_type].rb files listed above. These links will redirect to Code Climate. You need to fix these issues in each file, such as security, complexity, duplication, etc. And currently there is no unit tests for [question_type].rb. What needs to be done: 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 (“”).

�

What needs to be done: Refactor Criterion.rb 1. Method has too many lines: Line 35 - 158 [106/30] 2. Cyclomatic complexity for complete is too high: Line 35 - 158 [28/6] 3. Identical code found in 1 other location: Line 20 - 32 (mass = 89) 4. Identical code found in 1 other location: Line 135 - 139 (mass = 50) 5. Identical code found in 1 other location Line 13 - 14 (mass = 40) 6. Similar code found in 1 other location Line 103 - 105 (mass = 26) 7. Similar code found in 2 other locations Line 152 - 153 (mass = 22) 8. 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 Questionnaire_header.rb Upload_file.rb

Refactor

Criterion.rb

    1. Method has too many lines: Line 35 - 158 [106/30] 
      def complete(count, answer=nil, questionnaire_min, questionnaire_max, dropdown_or_scale)
       Wrap the specific code into different instance methods
    2. Cyclomatic complexity for complete is too high: Line 35 - 158 [28/6]
       def complete(count, answer=nil, questionnaire_min, questionnaire_max, dropdown_or_scale)
        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 = '

'

   #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

    3. Identical code found in 1 other location: Line 20 - 32 (mass = 89)
           Also found in   app/models/scale.rb:17…28
        The following method is removed from subclass: criterion and added to their superclass: scaled_question
      def view_question_text

def view_question_text

html = ' '+self.txt+' ' html += ''+self.type+'' html += ''+self.weight.to_s+''

   questionnaire = self.questionnaire
   if !self.max_label.nil? && !self.min_label.nil?

html += ' ('+self.min_label+') '+questionnaire.min_question_score.to_s+' to '+ questionnaire.max_question_score.to_s + ' ('+self.max_label+')'

   else

html += ''+questionnaire.min_question_score.to_s+' to '+ questionnaire.max_question_score.to_s + ''

end

html += '' Html.html_safe end 4. Identical code found in 1 other location: Line 135 - 139 (mass = 50) Also found in app/models/scale.rb:50…54 Wrap the duplicated code into an instance method and add the instance method into superclass: scaled_question

add the following functions to refactor duplicates.

def complete_questionnaire_min_to_questionnaire_max(ob, html, answer, questionnaire_min, questionnaire_max) for j in questionnaire_min..questionnaire_max html += '<input type="radio" id="' +j.to_s+ '" value="' +j.to_s+ '" name="Radio_' +ob.id.to_s+ '"'

     html += 'checked="checked"' if (!answer.nil? and answer.answer == j) or (answer.nil? and questionnaire_min == j)

html += '>' end return html end 5. Identical code found in 1 other location Line 13 - 14 (mass = 40) Also found in app/models/scale.rb:10…11 Wrap the duplicated code into an instance method and add the instance method into superclass: scaled_question # add the following functions to refactor duplicates. def edit_end(ob, html) html+=' max_label <input size="4" value="'+ob.max_label.to_s+'" name="question['+ob.id.to_s+'][max_label]" id="question_'+ob.id.to_s+'_max_label" type="text"> min_label <input size="4" value="'+ob.min_label.to_s+'" name="question['+ob.id.to_s+'][min_label]" id="question_'+ob.id.to_s+'_min_label" type="text">' html+='' end 6. Similar code found in 1 other location Line 103 - 105 (mass = 26) Also found in app/models/criterion.rb:107…110 Wrap the duplicated code into an instance method def complete_drop_down_label_config(html, label) html += j.to_s html += "-" + label if label && label.length>0 html += "</option>" end 7. Similar code found in 2 other locations Line 152 - 153 (mass = 22) Also found in app/models/criterion.rb:115…116 app/models/text_area.rb:12…13 Wrap the duplicated code into an instance method def complete_answer_comment(count, html, answer) 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? end 8. Similar code found in 3 other locations: Line 130 - 134 (mass = 18) Also found in app/models/criterion.rb:145…149 app/models/scale.rb:45…49 app/models/scale.rb:60…64 Wrap the duplicated code into an instance method and add the instance method into superclass: scaled_question # add the following functions to refactor duplicates. def complete_label(label, html) if !label.nil? html += '' +label+ ''

   else

html += ''

   end
 end

Scale.rb Cyclomatic complexity for complete is too high. Method has too many lines

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

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

 	html = self.txt + '
'
   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 += '<input id="responses_' +count.to_s+ '_comments" name="responses[' +count.to_s+ '][comment]" type="hidden" value="">'

html += '' html += ''

   for j in questionnaire_min..questionnaire_max

html += ''

end

html += ''

   # Identical code found in 1 other location, replaced with a method
   # if !self.min_label.nil?

# html += ''

   # else

# html += ''

   # end
   html = complete_label(self.min_label, html)

   # comment by Hui, replaced with a method
   # for j in questionnaire_min..questionnaire_max

# html += '' # end 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>' # comment by Hui, replaced with a method # if !self.max_label.nil? # html += ''

   # else

# html += ''

   # end
   html = complete_label(self.max_label, 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.html_safe
 end

Identical code found in 1 other location: line 17-28 app/models/criterion.rb:20…32

def view_question_text

The following method is removed from subclass: scale and added to their superclass: scaled_question

 #This method returns what to display if an instructor (etc.) is viewing a questionnaire
 def view_question_text

html = ' '+self.txt+' ' html += ''+self.type+'' html += ''+self.weight.to_s+''

   questionnaire = self.questionnaire
   if !self.max_label.nil? && !self.min_label.nil?

html += ' ('+self.min_label+') '+questionnaire.min_question_score.to_s+' to '+ questionnaire.max_question_score.to_s + ' ('+self.max_label+')'

   else

html += ''+questionnaire.min_question_score.to_s+' to '+ questionnaire.max_question_score.to_s + ''

end

html += '' html.html_safe end Identical code found in 1 other location: line 50-54 app/models/criterion.rb:135…139 Wrap the duplicated code into an instance method and add the instance method into superclass: scaled_question # add the following functions to refactor duplicates. def complete_questionnaire_min_to_questionnaire_max(ob, html, answer, questionnaire_min, questionnaire_max) for j in questionnaire_min..questionnaire_max html += '<input type="radio" id="' +j.to_s+ '" value="' +j.to_s+ '" name="Radio_' +ob.id.to_s+ '"'

     html += 'checked="checked"' if (!answer.nil? and answer.answer == j) or (answer.nil? and questionnaire_min == j)

html += '>' end return html end Identical code found in 1 other location: line 10-11 app/models/criterion.rb:13…14 Wrap the duplicated code into an instance method and add the instance method into superclass: scaled_question # add the following functions to refactor duplicates. def edit_end(ob, html) html+=' max_label <input size="4" value="'+ob.max_label.to_s+'" name="question['+ob.id.to_s+'][max_label]" id="question_'+ob.id.to_s+'_max_label" type="text"> min_label <input size="4" value="'+ob.min_label.to_s+'" name="question['+ob.id.to_s+'][min_label]" id="question_'+ob.id.to_s+'_min_label" type="text">' html+='' end Similar code found in 3 other locations: line 60-64 app/models/criterion.rb:130…134 app/models/criterion.rb:145…149 app/models/scale.rb:45…49 Wrap the duplicated code into an instance method and add the instance method into superclass: scaled_question # add the following functions to refactor duplicates. def complete_label(label, html) if !label.nil? html += '' +label+ ''

   else

html += ''

   end
 end

Checkbox.rb Method has too many lines. By moving codes to private methods outside this method, this problem can be fixed together with the following one. Here is the new complete method together with two additional private methods: hasAnswer(html, answer) and nextQuestionTail(html, next_question). The source codes are listed as below.

def complete(count, answer=nil)

   curr_question = Question.find(self.id)
   prev_question = Question.where("seq < ?", curr_question.seq).order(:seq).last
   next_question = Question.where("seq > ?", curr_question.seq).order(:seq).first
   html = 
   if prev_question.type == 'ColumnHeader'

html = ''

   #else
   #  html = 
   end

   html += '<input id="responses_' +count.to_s+ '_comments" name="responses[' +count.to_s+ '][comment]" type="hidden" value="">'
   html += '<input id="responses_' +count.to_s+ '_score" name="responses[' +count.to_s+ '][score]" type="hidden"'
   html += hasAnswer(html , answer)

if !answer.nil? and answer.answer == 1
html += 'value="1"'
else
html += 'value="0"'
end

   html += '>'
   html += '<input id="responses_' +count.to_s+ '_checkbox" type="checkbox" onchange="checkbox' +count.to_s+ 'Changed()"'
   html += 'checked="checked"' if !answer.nil? and answer.answer == 1
   html += '>'
   html += '<label for="responses_' +count.to_s+ '">' +self.txt+ '</label>'

   html += '<script>function checkbox' +count.to_s+ 'Changed() {'
   html += ' var checkbox = jQuery("#responses_' +count.to_s+ '_checkbox");'
   html += ' var response_score = jQuery("#responses_' +count.to_s+ '_score");'
   html += 'if (checkbox.is(":checked")) {'
   html += 'response_score.val("1");'
   html += '} else {'
   html += 'response_score.val("0");}}</script>'

   html = nextQuestionTail(html, next_question)

if next_question.type == 'ColumnHeader'
html += ''
elsif next_question.type == 'SectionHeader' or next_question.type == 'TableHeader'
html += '
'
else
html += '
'
end

   html.html_safe
 end

 #YJ private for complete()
 def hasAnswer(html, answer)
   if !answer.nil? and answer.answer == 1
     html += 'value="1"'
   else
     html += 'value="0"'
   end
   return html
 end

 def nextQuestionTail(html, next_q)
   if next_question.type == 'ColumnHeader'

html += '' elsif next_question.type == 'SectionHeader' or next_question.type == 'TableHeader' html += '
'

   else
     html += '
'
   end
   return html
 end

Cyclomatic complexity for complete is too high. This problem can be solved by moving branches outside to other private methods. See source code above. Similar codes in edit method are found in 2 other locations. (questionnaire_header.rb and upload_file.rb) Such code duplication can be eliminated by moving the identical part of the similar edit method to a new function, named edit_prefix(html, count) to their common superclass, which is Question (question.rb). Here is the source code of the added edit_prefix method in question.rb:

def edit_prefix(ob, html)

html ='' html+='<a rel="nofollow" data-method="delete" href="/questions/' +ob.id.to_s+ '">Remove</a>' html+='<input size="6" value="'+ob.seq.to_s+'" name="question['+ob.id.to_s+'][seq]" id="question_'+ob.id.to_s+'_seq" type="text">' html+='<textarea cols="50" rows="1" name="question['+ob.id.to_s+'][txt]" id="question_'+ob.id.to_s+'_txt">'+ob.txt+'</textarea>' html+='<input size="10" disabled="disabled" value="'+ob.type+'" name="question['+ob.id.to_s+'][type]" id="question_'+ob.id.to_s+'_type" type="text">'

end

The original code are commented out and insert the function call of the above function, html will be returned in the above function, see codes below:

def edit(count)

    html = edit_prefix(self, html)

html =''
html+='<a rel="nofollow" data-method="delete" href="/questions/' +self.id.to_s+ '">Remove</a>'
html+='<input size="6" value="'+self.seq.to_s+'" name="question['+self.id.to_s+'][seq]" id="question_'+self.id.to_s+'_seq" type="text">'
html+='<textarea cols="50" rows="1" name="question['+self.id.to_s+'][txt]" id="question_'+self.id.to_s+'_txt">'+self.txt+'</textarea>'
html+='<input size="10" disabled="disabled" value="'+self.type+'" name="question['+self.id.to_s+'][type]" id="question_'+self.id.to_s+'_type" type="text">'

html+='' html+='' html.html_safe end Similar codes in view_question_text method are found in 4 other locations. (dropdown.rb, questionnaire_header.rb, text_response.rb, upload_file.rb) The solution to this problem is the same as above one, below is the function added in the most common superclass, which is Question, that contains the common parts of the codes found in the view_question_text methods in all five classes. def view_qt_prefix(ob, html) html = ' '+ob.txt+ ' ' html += ''+ob.type+'' html += ''+ob.weight.to_s+''

end

The original code are commented out and insert the function call of the above function, html will be returned in the above function, see codes below:

def view_question_text

   html = view_qt_prefix(ob, html)

html = ' '+self.txt+' '
html += ''+self.type+''
html += ''+self.weight.to_s+''

html += 'Checked/Unchecked' html += '' html.html_safe end Questionnaire_header.rb All problems in this file are solved after solving the problems found in checkbox.rb. Upload_file.rb All problems in this file are solved after solving the problems found in checkbox.rb. Unit-test Criterion.rb Scale.rb Checkbox.rb Questionnaire_header.rb Upload_file.rb

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

Navigation menu

Search