CSC/ECE 517 Spring 2016/Refactor and write unit tests for question type.rb: Difference between revisions
No edit summary |
No edit summary |
||
Line 40: | Line 40: | ||
===Criterion.rb=== | ===Criterion.rb=== | ||
* Method has too many lines: Line 35 - 158 [106/30] | * 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] | * Cyclomatic complexity for complete is too high: Line 35 - 158 [28/6] | ||
Wrap the code into different instance methods to reduce cyclomatic complexity | |||
<code> | |||
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 | |||
</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:31, 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
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
- Scored question
- 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