CSC517 (Spring 2020) - E2010. Refactor criterion.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
No edit summary
 
(51 intermediate revisions by 3 users not shown)
Line 5: Line 5:
== About Expertiza ==
== About Expertiza ==


[http://expertiza.ncsu.edu/ Expertiza] is an open source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.
[http://expertiza.ncsu.edu/ Expertiza] is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.


== Problem Statement ==
== Problem Statement ==
Line 17: Line 17:
The ''criterion.rb'' model consists of functions that decide what to display while creating, editing and viewing the questionnaires depending on whether the user is an instructor, a teaching assistant or a student. So this file consists of a lot of HTML code that is rendered to the final questionnaire view.
The ''criterion.rb'' model consists of functions that decide what to display while creating, editing and viewing the questionnaires depending on whether the user is an instructor, a teaching assistant or a student. So this file consists of a lot of HTML code that is rendered to the final questionnaire view.


Criterion is a question in questionnaire. Other type question in questionnaire include Drop down (multiple choice), Text box (short question), Text area (long question). They can be add in questionnaire.
For this project, we have to deal with the following two methods:
For this project, we have to deal with the following two methods:
* The '''''complete''''' method, which is 104 lines long. this method returns the display for the students when they are filling the questionnaire. It includes the advice given for different questions, dropdown options to rate a project based on the question, a textarea to enter comments and so on.
* The ''complete'' method, which is 104 lines long. this method returns the display for the students when they are filling the questionnaire. It includes the advice given for different questions, dropdown options to rate a project based on the question, a textarea to enter comments and so on.
<pre>
<pre>
   def complete(count, answer = nil, questionnaire_min, questionnaire_max, dropdown_or_scale)
   def complete(count, answer = nil, questionnaire_min, questionnaire_max, dropdown_or_scale)
Line 144: Line 145:
   end
   end
</pre>
</pre>
* The '''''view_completed_question''''' method, which is 47 lies long. Thismethod is responsible to return the display if a student is viewingan already filled-out questionnaire.
* The ''view_completed_question'' method, which is 47 lies long. Thismethod is responsible to return the display if a student is viewingan already filled-out questionnaire.
<pre>
<pre>
def view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments = nil, current_user = nil)
def view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments = nil, current_user = nil)
Line 203: Line 204:
end
end
</pre>
</pre>


== Proposed Solution ==
== Proposed Solution ==
Our goal is to refactor the complete and view_completed_question method mainly by reducing the number of lines code for each function and also by introducing new methods to make the code more modular. We will also try to reduce the branch condition size wherever possible and hence reduce the cyclomatic complexity for these two functions. We also plan to introduce comments wherever needed to make the code more understandable. The main aim is to reduce the number of lines and make the code more compact without affecting the readability of the code.
Our goal is to refactor the complete and view_completed_question method mainly by reducing the number of lines code for each function and also by introducing new methods to make the code more modular.  
We will also try to reduce the branch condition size wherever possible and hence reduce the cyclomatic complexity for these two functions.  
We also plan to introduce comments wherever needed to make the code more understandable. The main aim is to reduce the number of lines and make the code more compact without affecting the readability of the code.
* High branch condition size problem: extracting three methods from method "'''complete'''"
** Method "''dropdown_criterion_question''" return html when choosing dropdown options
** Method "''scale_criterion_question''" return html when choosing scale options
** Method "''advices_criterion_question''" return html about showing advice for each criterion question
* Too long code
** Combining the short HTML strings into longer ones but not too long
** Using one readable line of code instead of three or more lines of if-else statements
* Fix incomplete condition problem
* Change the language to more Ruby friendly


== Implementation ==
== Implementation ==
Line 213: Line 224:


=== The ''complete'' method ===
=== The ''complete'' method ===
* We extracted three methods from "complete" method.
* We combined the short HTML strings into longer ones.
* We used the shorthand which beautifully consolidates three or more lines of code (read: if-else statements) into one readable line of code.


All the code in the model is just HTML. We thought of removing the code from here and putting it in a .erb file but it didn’t make much sense as the HTML code was heavily branched by the if-else conditions and it would just have been taking it out from this function and putting it in another file. We instead decided to refactor it in the file. We mainly combined the short HTML strings into longer ones but not too long so that the Code Climate cannot throw a “line too long” error. Another thing we did is reduce the number of lines used by condition statements by using the shorthand which beautifully consolidates three or more lines of code (read: if-else statements) into one readable line of code. (We also created another method for the repeated HTML code, so we just have to call the method rather than repeating the code. It increases code re-usability and reduces the number of lines of code.)
====''Examples of changes''====
'''Extracting three methods to reducing the condition branch'''
* Method "dropdown_criterion_question" return html when choosing dropdown options
* Method "scale_criterion_question" return html when choosing scale options
* Method "advices_criterion_question" return html about showing advice for each criterion question


====''List of changes''====
'''Combining the short HTML strings into longer ones'''


* Combining the short HTML strings into longer ones
We go from:
[[File:Combine1.png]]
<pre>
* Using one readable line of code instead of three or more lines of if-else statements
html += "</select></div><br><br>"
[[File:Oneline1.png]]
html += '<textarea' + ' id="responses_' + count.to_s + '_comments"'
</pre>
To:
<pre>
html += '</select></div><br><br><textarea' + ' id="responses_' + count.to_s + '_comments"'
</pre>
'''Using one readable line of code instead of three or more lines of if-else statements'''
 
We go from:
<pre>
html += if !answer.nil? and j == answer.answer
          '<option value=' + j.to_s + ' selected="selected">'
        else
          '<option value=' + j.to_s + '>'
        end
html += j.to_s
</pre>
To:
<pre>
html += '<option value=' + j.to_s
html += ' selected="selected"' if !answer.nil? && j == answer.answer
html += '>' + j.to_s
</pre>
 
'''Use unless for negative conditions'''
 
We go from:
<pre>
current_value += 'data-current-rating =' + answer.answer.to_s if !answer.nil?
</pre>
To:
<pre>
current_value += 'data-current-rating =' + answer.answer.to_s unless answer.nil?
</pre>
 
Same change for self.min_label and self.max_label. We go from:
<pre>
html += if !self.min_label.nil?
                '<td width="10%">' + self.min_label + '</td>'
              else
                '<td width="10%"></td>'
              end
</pre>
To:
<pre>
html += '<td width="10%">'
html += self.min_label unless self.min_label.nil?
html += '</td>'
</pre>
 
'''Unsafe condition'''
 
We added condition to make sure when answer.comments is nil. We go from:
<pre>
html += answer.comments unless answer.nil?
</pre>
To:
<pre>
html += answer.comments if !answer.nil? && !answer.comments.nil?
</pre>
 
'''Incomplete condition'''
 
When create a new criterion, which size is "" that isn't nil. We add condition to make sure "cols" and "rows" will be assign when "self.size" is "". We go from:
<pre>
if self.size.nil?
  cols = '70'
  rows = '1'
else
  cols = self.size.split(',')[0]
  rows = self.size.split(',')[1]
end
html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
        ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
</pre>
To:
<pre>
if self.size.nil?||self.size.blank?
  cols = '70'
  rows = '1'
else
  cols = self.size.split(',')[0]
  rows = self.size.split(',')[1]
end
html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
        ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
</pre>
 
'''Ruby grammer'''
 
We change "and" to &&. We changed from:
<pre>
html += 'checked="checked"' if (!answer.nil? and answer.answer == j) or (answer.nil? and questionnaire_min == j)
</pre>
To:
<pre>
html += 'checked="checked"' if (!answer.nil? && answer.answer == j) or (answer.nil? && questionnaire_min == j)
</pre>


====''The modified ''complete'' method''====
====''The modified ''complete'' method''====
* '''Method "complete"'''
<pre>
<pre>
   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?
    html = '<div><label for="responses_' + count.to_s + '">' + self.txt + '</label></div>'
    question_advices = QuestionAdvice.where(question_id: self.id).sort_by(&:id)
    advice_total_length = 0
    question_advices.each do |question_advice|
      advice_total_length += question_advice.advice.length if question_advice.advice && question_advice.advice != ""
    end
    #show advice given for different questions
    html += advices_criterion_question(count, question_advices) if !question_advices.empty? and advice_total_length > 0
 
    #dropdown options to rate a project based on the quetion
    html += dropdown_criterion_question(count, answer, questionnaire_min, questionnaire_max) if dropdown_or_scale == 'dropdown'
 
    #scale optioins
    html += scale_criterion_question(count, answer, questionnaire_min, questionnaire_max) if dropdown_or_scale == 'scale'
    safe_join(["".html_safe, "".html_safe], html.html_safe)
  end
</pre>
* '''Method "advices_criterion_question"'''
<pre>
  # show advice for each criterion question
  def advices_criterion_question(count, question_advices)
    html = '<a id="showAdvice_' + self.id.to_s + '" onclick="showAdvice(' + self.id.to_s + ')">Show advice</a><script>'
    html += 'function showAdvice(i){var element = document.getElementById("showAdivce_" + i.toString());'
    html += 'var show = element.innerHTML == "Hide advice";'
    html += 'if (show){element.innerHTML="Show advice";} else{element.innerHTML="Hide advice";}toggleAdvice(i);}'
    html += 'function toggleAdvice(i) {var elem = document.getElementById(i.toString() + "_myDiv");'
    html += 'if (elem.style.display == "none") {elem.style.display = "";} else {elem.style.display = "none";}}</script>'
    html += '<div id="' + self.id.to_s + '_myDiv" style="display: none;">'
    # [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 += (self.questionnaire.max_question_score - index).to_s + ' - ' + question_advice.advice + '</a><br/><script>'
      html += 'function changeScore(i, j) {var elem = jQuery("#responses_" + i.toString() + "_score");'
      html += 'var opts = elem.children("option").length;'
      html += 'elem.val((' + self.questionnaire.max_question_score.to_s + ' - j).toString());}</script>'
    end
    html += '</div>'
  end
</pre>
* '''Method "dropdown_criterion_question"'''
<pre>
  def dropdown_criterion_question(count, answer = nil, questionnaire_min, questionnaire_max)
    current_value = ""
    current_value += 'data-current-rating =' + answer.answer.to_s unless answer.nil?
    html = '<div><select id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" class="review-rating" ' + current_value + '>'
    html += "<option value = ''>--</option>"
    questionnaire_min.upto(questionnaire_max).each do |j|
      html += '<option value=' + j.to_s
      html += ' selected="selected"' if !answer.nil? && j == answer.answer
      html += '>'
      html += j.to_s
      html += "-" + self.min_label if self.min_label.present? && j == questionnaire_min
      html += "-" + self.max_label if self.max_label.present? && j == questionnaire_max
      html += "</option>"
    end
 
    html += '</select></div><br><br><textarea' + ' id="responses_' + count.to_s + '_comments"'
    html += ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
    html += answer.comments if !answer.nil? && !answer.comments.nil?
    html += '</textarea></td>'
  end
</pre>
* '''Method "scale_criterion_question"'''
<pre>
  def scale_criterion_question(count, answer = nil, questionnaire_min, questionnaire_max)
     if self.size.nil? || !self.size.present?
       cols = '70'
       cols = '70'
       rows = '1'
       rows = '1'
     else
     else
      cols = self.size.split(',')[0]
    cols = self.size.split(',')[0]
      rows = self.size.split(',')[1]
    rows = self.size.split(',')[1]
    end
    html = '<input id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" type="hidden"'
    html += 'value="' + answer.answer.to_s + '"' unless answer.nil?
    html += '><table><tr><td width="10%"></td>'
 
    (questionnaire_min..questionnaire_max).each do |j|
      html += '<td width="10%"><label>' + j.to_s + '</label></td>'
     end
     end


     html = '<div><label for="responses_' + count.to_s + '">' + self.txt + '</label></div>'
     html += '<td width="10%"></td></tr><tr><td width="10%">'
     # show advice for each criterion question
     html += self.min_label unless self.min_label.nil?
    question_advices = QuestionAdvice.where(question_id: self.id).sort_by(&:id)
     html += '</td>'
     advice_total_length = 0


     question_advices.each do |question_advice|
     (questionnaire_min..questionnaire_max).each do |j|
       advice_total_length += question_advice.advice.length if question_advice.advice && question_advice.advice != ""
       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? && answer.answer == j) or (answer.nil? && questionnaire_min == j)
      html += '></td>'
     end
     end
    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><td width="10%">'
    html += self.max_label unless self.max_label.nil?
    html += '</td><td width="10%"></td></tr></table>'
    html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
      ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
    html += answer.comments if !answer.nil? && !answer.comments.nil?
    html += '</textarea>'   
  end
</pre>
=== The ''view_completed_question" method===


    if !question_advices.empty? and advice_total_length > 0
The main task here was to reduce the nested if-else branching and reduce the number of arguments for this function.  
      html += '<a id="showAdvice_' + self.id.to_s + '" onclick="showAdvice(' + self.id.to_s + ')">Show advice</a><script>'
* We reduced the nested if-else branching.
      html += 'function showAdvice(i){var element = document.getElementById("showAdivce_" + i.toString());'
* We concatenated small HTML string
      html += 'var show = element.innerHTML == "Hide advice";'
* We used shorthand for if-else condition statements.  
      html += 'if (show){element.innerHTML="Show advice";} else{element.innerHTML="Hide advice";}toggleAdvice(i);}'


      html += 'function toggleAdvice(i) {var elem = document.getElementById(i.toString() + "_myDiv");'
This was a very small method with its major dependent functions present in response.rb file. To refactor this method, we would have had to refactor response.rb and in turn, the other question subclasses which would make it very complex and break the code. According to us, refactoring this method along with the response.rb file would make more sense.  
      html += 'if (elem.style.display == "none") {elem.style.display = "";} else {elem.style.display = "none";}}</script>'


      html += '<div id="' + self.id.to_s + '_myDiv" style="display: none;">'
====''Example of changes''====
      # [2015-10-26] Zhewei:
'''Combining the short HTML strings into longer ones'''
      # 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 += (self.questionnaire.max_question_score - index).to_s + ' - ' + question_advice.advice + '</a><br/><script>'
        html += 'function changeScore(i, j) {var elem = jQuery("#responses_" + i.toString() + "_score");'
        html += 'var opts = elem.children("option").length;'
        html += 'elem.val((' + self.questionnaire.max_question_score.to_s + ' - j).toString());}</script>'
      end
      html += '</div>'
    end


    if dropdown_or_scale == 'dropdown'
We concatenated small HTML which was from:
      current_value = ""
<pre>
      current_value += 'data-current-rating =' + answer.answer.to_s if !answer.nil?
html += '<table cellpadding="5">'
html += '<tr>'
html += '<td>'
</pre>
To:
<pre>
html += '<table cellpadding="5"><tr><td>'
</pre>


      html += '<div><select id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" class="review-rating" ' + current_value + '>' + "<option value = ''>--</option>" + '<option value='
'''Using one readable line of code instead of three or more lines of if-else statements'''
      questionnaire_min.upto(questionnaire_max).each do |j|
        html += j.to_s + 'selected="selected"' if !answer.nil? and j == answer.answer + '>'       
        html += j.to_s + "-"
        html += self.min_label if self.min_label.present? and j == questionnaire_min + "</option>"
        html += self.max_label if self.max_label.present? and j == questionnaire_max + "</option>"
      end
      html += '</select></div><br><br><textarea' + ' id="responses_' + count.to_s + '_comments"' \
      ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
      html += answer.comments unless answer.nil? + '</textarea></td>'
    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 + '"' unless answer.nil? + '><table><tr><td width="10%"></td>'


      (questionnaire_min..questionnaire_max).each do |j|
We changed from:
        html += '<td width="10%"><label>' + j.to_s + '</label></td>'
<pre>
      end
score_percent = if score != "-"
      html += '<td width="10%"></td></tr><tr><td width="10%">' + self.min_label if !self.min_label.nil? + '</td>'
                    answer.answer * 1.0 / questionnaire_max
                else
                    0
                end
</pre>
To:
<pre>
score_percent = score != "-" ? answer.answer * 1.0 / questionnaire_max : 0
</pre>


      (questionnaire_min..questionnaire_max).each do |j|
'''Delete useless variable - 'resp''''
        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) + '></td>'
      end
      html += '<script>jQuery("input[name=Radio_' + self.id.to_s + ']:radio").change(function() {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><td width="10%">'


      html += self.max_label if !self.max_label.nil? + '</td><td width="10%"></td></tr></table>'
We deleted this code
      html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
<pre>
        ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
resp = Response.find(answer.response_id)
      html += answer.comments unless answer.nil? + '</textarea>'
    end
    safe_join(["".html_safe, "".html_safe], html.html_safe)
  end
</pre>
</pre>


====The ''view_completed_question" method====
'''Change of language to make it more Ruby friendly'''
 
We changed from:
<pre>
if tag_dep.question_type == question.type and answer.comments.length > tag_dep.answer_length_threshold.to_i
</pre>
To:
<pre>
if tag_dep.question_type == question.type && answer.comments.length > tag_dep.answer_length_threshold.to_i
</pre>
'''Reduce the branch condition'''


The main task here was to reduce the nested if-else branching and reduce the number of arguments for this function. This was a very small method with its major dependent functions present in response.rb file. To refactor this method, we would have had to refactor response.rb and in turn, the other question subclasses which would make it very complex and break the code. According to us, refactoring this method along with the response.rb file would make more sense. We still made some changes in the file like concatenating small HTML string and using shorthand for if-else condition statements.  
We combined condition
<pre>
unless tag_prompt_deployments.nil?
</pre>
and
<pre>
if tag_prompt_deployments.count > 0
</pre>
To:
<pre>
if !tag_prompt_deployments.nil? && tag_prompt_deployments.count > 0
</pre>
  [[File:Multiple_2.png]]


The modified ''view_completed_question'' method is as below:
==== The modified ''view_completed_question''====


<pre>
<pre>
   def view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments = nil, current_user = nil)
   def view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments = nil, current_user = nil)
     html = '<b>' + count.to_s + ". " + self.txt + ' [Max points: ' + questionnaire_max.to_s + "]</b>"
     html = '<b>' + count.to_s + ". " + self.txt + ' [Max points: ' + questionnaire_max.to_s + "]</b>"
     score = answer && !answer.answer.nil? ? answer.answer.to_s : "-"
     score = answer && !answer.answer.nil? ? answer.answer.to_s : "-"
     score_percent = score != "-" ? answer.answer * 1.0 / questionnaire_max : 0
     score_percent = score != "-" ? answer.answer * 1.0 / questionnaire_max : 0
Line 339: Line 545:
       html += '<td style="padding-left:10px"><br>' + answer.comments.html_safe + '</td>'
       html += '<td style="padding-left:10px"><br>' + answer.comments.html_safe + '</td>'
       #### start code to show tag prompts ####
       #### start code to show tag prompts ####
       unless tag_prompt_deployments.nil?
       if !tag_prompt_deployments.nil? && tag_prompt_deployments.count > 0
         # show check boxes for answer tagging
         # show check boxes for answer tagging
        resp = Response.find(answer.response_id)
         question = Question.find(answer.question_id)
         question = Question.find(answer.question_id)
         if tag_prompt_deployments.count > 0
         html += '<tr><td colspan="2">'
          html += '<tr><td colspan="2">'
        tag_prompt_deployments.each do |tag_dep|
          tag_prompt_deployments.each do |tag_dep|
          tag_prompt = TagPrompt.find(tag_dep.tag_prompt_id)
            tag_prompt = TagPrompt.find(tag_dep.tag_prompt_id)
          if tag_dep.question_type == question.type && answer.comments.length > tag_dep.answer_length_threshold.to_i
            if tag_dep.question_type == question.type and answer.comments.length > tag_dep.answer_length_threshold.to_i
            html += tag_prompt.html_control(tag_dep, answer, current_user)
              html += tag_prompt.html_control(tag_dep, answer, current_user)
            end
           end
           end
          html += '</td></tr>'
         end
         end
        html += '</td></tr>'
       end
       end
       #### end code to show tag prompts ####
       #### end code to show tag prompts ####
Line 361: Line 564:
</pre>
</pre>


== Testing ==
== Test Plan ==
 
We are running Rspec tests to make sure the coverage is the same and manually checking the pages to make sure there are not any issues and the code is not breaking
=== RSpec Testing ===
The origin test about method "complete" didn't include the situation when the parameter "answer" isn't nil and the parameter "dropdown_or_scale" is "dropdown" or "scale". We added the test examples from 4 to 16 including the multiple for method "complete", "dropdown_criterion_question", "scale_criterion_question".
* '''Test for method "complete" with or without answer and dorpdown_or_scale'''
'''Origin test'''
<pre>
describe "#complete" do
    it "returns the html " do
      html = criterion.complete(0, nil, 0, 5).to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div>")
    end
  end
</pre>
 
'''New test'''
<pre>
describe "#complete" do
    it "returns the html without answer and no dropdown or scale" do
      html = criterion.complete(0, nil, 0, 5).to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div>")
    end
 
    it "returns the html without answer and dropdown" do
      html = criterion.complete(0, nil, 0, 5, dropdown_or_scale = 'dropdown').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" ><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end
 
    it "returns the html with no comments answer and answer.answer outside questionnaire min and max and dropdown" do
      html = criterion.complete(0, answer_no_comments, 0, 5, dropdown_or_scale = 'dropdown').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =8><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end
 
    it "returns the html with comments answer and answer.answer between questionnaire min and max and dropdown" do
      html = criterion.complete(0, answer_comments, 0, 5, dropdown_or_scale = 'dropdown').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =3><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3 selected=\"selected\">3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea></td>")
    end
 
    it "returns the html without answer and scale" do
      html = criterion.complete(0, nil, 0, 5, dropdown_or_scale = 'scale').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end
 
    it "returns the html with no comments answer and answer.answer outside questionnaire min and max and scale" do
      html = criterion.complete(0, answer_no_comments, 0, 5, dropdown_or_scale = 'scale').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"8\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end
 
    it "returns the html with comments answer and answer.answer between questionnaire min and max and scale" do
      html = criterion.complete(0, answer_comments, 0, 5, dropdown_or_scale = 'scale').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"3\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea>")
    end
 
  end
</pre>
 
* '''Test for method "dropdown_criterion_question" with or without answer'''
<pre>
describe "#dropdown_criterion_question" do
    it "returns the html without answer" do
      html = criterion.dropdown_criterion_question(0, nil, 0, 5).to_s
      expect(html).to eq("<div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" ><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end
   
    it "returns the html with no comments answer and answer.answer outside questionnaire min and max" do
      html = criterion.dropdown_criterion_question(0, answer_no_comments, 0, 5).to_s
      expect(html).to eq("<div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =8><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end
 
    it "returns the html with comments in answer and answer.answer between questionnaire min and max" do
      html = criterion.dropdown_criterion_question(0, answer_comments, 0, 5).to_s
      expect(html).to eq("<div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =3><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3 selected=\"selected\">3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea></td>")
    end
  end
</pre>
* '''Test for method "scale_criterion_question" with or without answer'''
<pre>
describe "#scale_criterion_question" do
    it "returns the html without answer" do
      html = criterion.scale_criterion_question(0, nil, 0, 5).to_s
      expect(html).to eq("<input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end
 
    it "returns the html with no comments answer and answer.answer outside questionnaire min and max" do
      html = criterion.scale_criterion_question(0, answer_no_comments, 0, 5).to_s
      expect(html).to eq("<input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"8\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end
 
    it "returns the html with comments answer and answer.answer between questionnaire min and max" do
      html = criterion.scale_criterion_question(0, answer_comments, 0, 5).to_s
      expect(html).to eq("<input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"3\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea>")
    end
  end
</pre>
 
* '''All existing tests passed'''
[[File:Test1.png]]
 
=== Manual Testing ===
'''Test Log in'''
Website url: http://152.46.19.120:8080/ <br>
Log in: instructor6<br>
Password: password<br>
Log in: student2064<br>
Password: password<br>
 
'''Test criterion works well in questionnaire'''
 
1. After logging in as a student, click the "Assignments"<br>
2. Choose an assignment, like "Backchannel application review"<br>
3. You can choose "Your scores" to see the review results<br>
4. Click Criterion 1, you can see all the reviews for this criterion.<br>
5. Click "toggle question list", you can see all criterion questions.<br>
6. After logging in as an instructor, choose "Questionnaires" tag<br>
7. Click on the name Review: This should show a drop-down showing different reviews made.<br>
8. Choose a review, select the edit icon to the right.<br>
8. Change dropbox of question type to Criterion, select Add "2" more. Press the add button.<br>
9. Edit question content: "Test question Textarea", "Test question Criterion1", and "Test question Criterion2" in that order.<br>
10. You can also change the question content for exited criterion question<br>
 
These manual tests show criterion works well. Feel free to try your own test cases.

Latest revision as of 04:16, 16 April 2020

E2010 Refactor criterion.rb

This page provides a description of the Expertiza based OSS project.

About Expertiza

Expertiza is an open-source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

Problem Statement

The criterion.rb model consists of functions that decide what to display while creating, editing and viewing the questionnaires depending on whether the user is an instructor, a teaching assistant or a student. So, this file consists of a lot of HTML code that is rendered to the final questionnaire view.

The current version of this controller has four methods, but two methods are very long and would require refactoring. The primary problem with these functions is that it consists of many statements of string concatenation. The HTML code which is rendered to the final view is made up of concatenations.

Another issue with the code is that the branch conditions size for the complete and the view_completed_question method is very high. Also, this controller has very few comments. They are specifically needed to differentiate between the purpose of numerous nested branches from each other.

About criterion.rb

The criterion.rb model consists of functions that decide what to display while creating, editing and viewing the questionnaires depending on whether the user is an instructor, a teaching assistant or a student. So this file consists of a lot of HTML code that is rendered to the final questionnaire view.

Criterion is a question in questionnaire. Other type question in questionnaire include Drop down (multiple choice), Text box (short question), Text area (long question). They can be add in questionnaire. For this project, we have to deal with the following two methods:

  • The complete method, which is 104 lines long. this method returns the display for the students when they are filling the questionnaire. It includes the advice given for different questions, dropdown options to rate a project based on the question, a textarea to enter comments and so on.
  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 = '<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(&:id)
    advice_total_length = 0

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

    if !question_advices.empty? 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;">'
      # [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 += (self.questionnaire.max_question_score - 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((' + self.questionnaire.max_question_score.to_s + ' - j).toString());}'
        html += '</script>'
      end
      html += '</div>'
    end

    if dropdown_or_scale == 'dropdown'
      current_value = ""
      current_value += 'data-current-rating =' + answer.answer.to_s if !answer.nil?
      html += '<div><select id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" class="review-rating" ' + current_value + '>'
      html += "<option value = ''>--</option>"
      questionnaire_min.upto(questionnaire_max).each do |j|
        html += if !answer.nil? and j == answer.answer
                  '<option value=' + j.to_s + ' selected="selected">'
                else
                  '<option value=' + j.to_s + '>'
                end

        html += j.to_s
        if j == questionnaire_min
          html += "-" + self.min_label if self.min_label.present?
        elsif j == questionnaire_max
          html += "-" + self.max_label if self.max_label.present?
        end
        html += "</option>"
      end
      html += "</select></div><br><br>"
      html += '<textarea' + ' id="responses_' + count.to_s + '_comments"' \
       ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
      html += answer.comments unless answer.nil?
      html += '</textarea></td>'
    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 + '"' unless answer.nil?
      html += '>'

      html += '<table>'
      html += '<tr><td width="10%"></td>'
      (questionnaire_min..questionnaire_max).each do |j|
        html += '<td width="10%"><label>' + j.to_s + '</label></td>'
      end
      html += '<td width="10%"></td></tr><tr>'

      html += if !self.min_label.nil?
                '<td width="10%">' + self.min_label + '</td>'
              else
                '<td width="10%"></td>'
              end
      (questionnaire_min..questionnaire_max).each do |j|
        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
      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>'

      html += if !self.max_label.nil?
                '<td width="10%">' + self.max_label + '</td>'
              else
                '<td width="10%"></td>'
              end

      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]" class="tinymce">'
      html += answer.comments unless answer.nil?
      html += '</textarea>'

    end
    safe_join(["".html_safe, "".html_safe], html.html_safe)
  end
  • The view_completed_question method, which is 47 lies long. Thismethod is responsible to return the display if a student is viewingan already filled-out questionnaire.
def view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments = nil, current_user = nil)
    html = '<b>' + count.to_s + ". " + self.txt + ' [Max points: ' + questionnaire_max.to_s + "]</b>"

    score = answer && !answer.answer.nil? ? answer.answer.to_s : "-"
    score_percent = if score != "-"
                      answer.answer * 1.0 / questionnaire_max
                    else
                      0
                    end

    score_color = if score_percent > 0.8
                    "c5"
                  elsif score_percent > 0.6
                    "c4"
                  elsif score_percent > 0.4
                    "c3"
                  elsif score_percent > 0.2
                    "c2"
                  else
                    "c1"
                  end

    html += '<table cellpadding="5">'
    html += '<tr>'
    html += '<td>'
    html += '<div class="' + score_color + '" style="width:30px; height:30px;' \
      ' border-radius:50%; font-size:15px; color:black; line-height:30px; text-align:center;">'
    html += score
    html += '</div>'
    html += '</td>'
    if answer && !answer.comments.nil?
      html += '<td style="padding-left:10px">'
      html += '<br>' + answer.comments.html_safe
      html += '</td>'
      #### start code to show tag prompts ####
      unless tag_prompt_deployments.nil?
        # show check boxes for answer tagging
        resp = Response.find(answer.response_id)
        question = Question.find(answer.question_id)
        if tag_prompt_deployments.count > 0
          html += '<tr><td colspan="2">'
          tag_prompt_deployments.each do |tag_dep|
            tag_prompt = TagPrompt.find(tag_dep.tag_prompt_id)
            if tag_dep.question_type == question.type and answer.comments.length > tag_dep.answer_length_threshold.to_i
              html += tag_prompt.html_control(tag_dep, answer, current_user)
            end
          end
          html += '</td></tr>'
        end
      end
      #### end code to show tag prompts ####
    end
    html += '</tr></table>'
    safe_join(["".html_safe, "".html_safe], html.html_safe)
  end
end

Proposed Solution

Our goal is to refactor the complete and view_completed_question method mainly by reducing the number of lines code for each function and also by introducing new methods to make the code more modular. We will also try to reduce the branch condition size wherever possible and hence reduce the cyclomatic complexity for these two functions. We also plan to introduce comments wherever needed to make the code more understandable. The main aim is to reduce the number of lines and make the code more compact without affecting the readability of the code.

  • High branch condition size problem: extracting three methods from method "complete"
    • Method "dropdown_criterion_question" return html when choosing dropdown options
    • Method "scale_criterion_question" return html when choosing scale options
    • Method "advices_criterion_question" return html about showing advice for each criterion question
  • Too long code
    • Combining the short HTML strings into longer ones but not too long
    • Using one readable line of code instead of three or more lines of if-else statements
  • Fix incomplete condition problem
  • Change the language to more Ruby friendly

Implementation

We tried to tackle the issues mentioned in the problem statement as described below:

The complete method

  • We extracted three methods from "complete" method.
  • We combined the short HTML strings into longer ones.
  • We used the shorthand which beautifully consolidates three or more lines of code (read: if-else statements) into one readable line of code.

Examples of changes

Extracting three methods to reducing the condition branch

  • Method "dropdown_criterion_question" return html when choosing dropdown options
  • Method "scale_criterion_question" return html when choosing scale options
  • Method "advices_criterion_question" return html about showing advice for each criterion question

Combining the short HTML strings into longer ones

We go from:

html += "</select></div><br><br>"
html += '<textarea' + ' id="responses_' + count.to_s + '_comments"'

To:

html += '</select></div><br><br><textarea' + ' id="responses_' + count.to_s + '_comments"'

Using one readable line of code instead of three or more lines of if-else statements

We go from:

html += if !answer.nil? and j == answer.answer
           '<option value=' + j.to_s + ' selected="selected">'
        else
           '<option value=' + j.to_s + '>'
        end
html += j.to_s

To:

html += '<option value=' + j.to_s
html += ' selected="selected"' if !answer.nil? && j == answer.answer
html += '>' + j.to_s

Use unless for negative conditions

We go from:

current_value += 'data-current-rating =' + answer.answer.to_s if !answer.nil?

To:

current_value += 'data-current-rating =' + answer.answer.to_s unless answer.nil?

Same change for self.min_label and self.max_label. We go from:

html += if !self.min_label.nil?
                '<td width="10%">' + self.min_label + '</td>'
              else
                '<td width="10%"></td>'
              end

To:

html += '<td width="10%">'
html += self.min_label unless self.min_label.nil?
html += '</td>'

Unsafe condition

We added condition to make sure when answer.comments is nil. We go from:

html += answer.comments unless answer.nil?

To:

html += answer.comments if !answer.nil? && !answer.comments.nil?

Incomplete condition

When create a new criterion, which size is "" that isn't nil. We add condition to make sure "cols" and "rows" will be assign when "self.size" is "". We go from:

if self.size.nil?
   cols = '70'
   rows = '1'
else
   cols = self.size.split(',')[0]
   rows = self.size.split(',')[1]
end
html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
        ' name="responses[' + count.to_s + '][comment]" class="tinymce">'

To:

if self.size.nil?||self.size.blank?
   cols = '70'
   rows = '1'
else
   cols = self.size.split(',')[0]
   rows = self.size.split(',')[1]
end
html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
        ' name="responses[' + count.to_s + '][comment]" class="tinymce">'

Ruby grammer

We change "and" to &&. We changed from:

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

To:

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

The modified complete method

  • Method "complete"
  def complete(count, answer = nil, questionnaire_min, questionnaire_max, dropdown_or_scale)
    html = '<div><label for="responses_' + count.to_s + '">' + self.txt + '</label></div>'
    question_advices = QuestionAdvice.where(question_id: self.id).sort_by(&:id)
    advice_total_length = 0
    question_advices.each do |question_advice|
      advice_total_length += question_advice.advice.length if question_advice.advice && question_advice.advice != ""
    end
    #show advice given for different questions
    html += advices_criterion_question(count, question_advices) if !question_advices.empty? and advice_total_length > 0

    #dropdown options to rate a project based on the quetion
    html += dropdown_criterion_question(count, answer, questionnaire_min, questionnaire_max) if dropdown_or_scale == 'dropdown'

    #scale optioins
    html += scale_criterion_question(count, answer, questionnaire_min, questionnaire_max) if dropdown_or_scale == 'scale'
    safe_join(["".html_safe, "".html_safe], html.html_safe)
  end
  • Method "advices_criterion_question"
  # show advice for each criterion question
  def advices_criterion_question(count, question_advices)
    html = '<a id="showAdvice_' + self.id.to_s + '" onclick="showAdvice(' + self.id.to_s + ')">Show advice</a><script>'
    html += 'function showAdvice(i){var element = document.getElementById("showAdivce_" + i.toString());'
    html += 'var show = element.innerHTML == "Hide advice";'
    html += 'if (show){element.innerHTML="Show advice";} else{element.innerHTML="Hide advice";}toggleAdvice(i);}'
    html += 'function toggleAdvice(i) {var elem = document.getElementById(i.toString() + "_myDiv");'
    html += 'if (elem.style.display == "none") {elem.style.display = "";} else {elem.style.display = "none";}}</script>'
    html += '<div id="' + self.id.to_s + '_myDiv" style="display: none;">'
    # [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 += (self.questionnaire.max_question_score - index).to_s + ' - ' + question_advice.advice + '</a><br/><script>'
      html += 'function changeScore(i, j) {var elem = jQuery("#responses_" + i.toString() + "_score");'
      html += 'var opts = elem.children("option").length;'
      html += 'elem.val((' + self.questionnaire.max_question_score.to_s + ' - j).toString());}</script>'
    end
    html += '</div>'
  end
  • Method "dropdown_criterion_question"
  def dropdown_criterion_question(count, answer = nil, questionnaire_min, questionnaire_max)
    current_value = "" 
    current_value += 'data-current-rating =' + answer.answer.to_s unless answer.nil?
    html = '<div><select id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" class="review-rating" ' + current_value + '>'
    html += "<option value = ''>--</option>"
    questionnaire_min.upto(questionnaire_max).each do |j|
      html += '<option value=' + j.to_s
      html += ' selected="selected"' if !answer.nil? && j == answer.answer
      html += '>'
      html += j.to_s
      html += "-" + self.min_label if self.min_label.present? && j == questionnaire_min
      html += "-" + self.max_label if self.max_label.present? && j == questionnaire_max
      html += "</option>"
    end

    html += '</select></div><br><br><textarea' + ' id="responses_' + count.to_s + '_comments"'
    html += ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
    html += answer.comments if !answer.nil? && !answer.comments.nil?
    html += '</textarea></td>'
  end
  • Method "scale_criterion_question"
  def scale_criterion_question(count, answer = nil, questionnaire_min, questionnaire_max)
    if self.size.nil? || !self.size.present?
      cols = '70'
      rows = '1'
    else
     cols = self.size.split(',')[0]
     rows = self.size.split(',')[1]
    end
    html = '<input id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" type="hidden"'
    html += 'value="' + answer.answer.to_s + '"' unless answer.nil?
    html += '><table><tr><td width="10%"></td>'

    (questionnaire_min..questionnaire_max).each do |j|
      html += '<td width="10%"><label>' + j.to_s + '</label></td>'
    end

    html += '<td width="10%"></td></tr><tr><td width="10%">'
    html += self.min_label unless self.min_label.nil?
    html += '</td>'

    (questionnaire_min..questionnaire_max).each do |j|
      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? && answer.answer == j) or (answer.nil? && questionnaire_min == j)
      html += '></td>'
    end
    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><td width="10%">'
    html += self.max_label unless self.max_label.nil?
    html += '</td><td width="10%"></td></tr></table>'
    html += '<textarea cols=' + cols + ' rows=' + rows + ' id="responses_' + count.to_s + '_comments"' \
      ' name="responses[' + count.to_s + '][comment]" class="tinymce">'
    html += answer.comments if !answer.nil? && !answer.comments.nil?
    html += '</textarea>'    
  end

The view_completed_question" method

The main task here was to reduce the nested if-else branching and reduce the number of arguments for this function.

  • We reduced the nested if-else branching.
  • We concatenated small HTML string
  • We used shorthand for if-else condition statements.

This was a very small method with its major dependent functions present in response.rb file. To refactor this method, we would have had to refactor response.rb and in turn, the other question subclasses which would make it very complex and break the code. According to us, refactoring this method along with the response.rb file would make more sense.

Example of changes

Combining the short HTML strings into longer ones

We concatenated small HTML which was from:

html += '<table cellpadding="5">'
html += '<tr>'
html += '<td>'

To:

html += '<table cellpadding="5"><tr><td>'

Using one readable line of code instead of three or more lines of if-else statements

We changed from:

score_percent = if score != "-"
                     answer.answer * 1.0 / questionnaire_max
                else
                     0
                end

To:

score_percent = score != "-" ? answer.answer * 1.0 / questionnaire_max : 0

Delete useless variable - 'resp'

We deleted this code

resp = Response.find(answer.response_id)

Change of language to make it more Ruby friendly

We changed from:

if tag_dep.question_type == question.type and answer.comments.length > tag_dep.answer_length_threshold.to_i

To:

if tag_dep.question_type == question.type && answer.comments.length > tag_dep.answer_length_threshold.to_i

Reduce the branch condition

We combined condition

unless tag_prompt_deployments.nil?

and

if tag_prompt_deployments.count > 0

To:

if !tag_prompt_deployments.nil? && tag_prompt_deployments.count > 0
 

The modified view_completed_question

  def view_completed_question(count, answer, questionnaire_max, tag_prompt_deployments = nil, current_user = nil)
    html = '<b>' + count.to_s + ". " + self.txt + ' [Max points: ' + questionnaire_max.to_s + "]</b>"
    score = answer && !answer.answer.nil? ? answer.answer.to_s : "-"
    score_percent = score != "-" ? answer.answer * 1.0 / questionnaire_max : 0
    score_color = if score_percent > 0.8
                    "c5"
                  elsif score_percent > 0.6
                    "c4"
                  elsif score_percent > 0.4
                    "c3"
                  elsif score_percent > 0.2
                    "c2"
                  else
                    "c1"
                  end

    html += '<table cellpadding="5"><tr><td>'
    html += '<div class="' + score_color + '" style="width:30px; height:30px;' \
      ' border-radius:50%; font-size:15px; color:black; line-height:30px; text-align:center;">'
    html += score + '</div></td>'

    if answer && !answer.comments.nil?
      html += '<td style="padding-left:10px"><br>' + answer.comments.html_safe + '</td>'
      #### start code to show tag prompts ####
      if !tag_prompt_deployments.nil? && tag_prompt_deployments.count > 0
        # show check boxes for answer tagging
        question = Question.find(answer.question_id)
        html += '<tr><td colspan="2">'
        tag_prompt_deployments.each do |tag_dep|
          tag_prompt = TagPrompt.find(tag_dep.tag_prompt_id)
          if tag_dep.question_type == question.type && answer.comments.length > tag_dep.answer_length_threshold.to_i
            html += tag_prompt.html_control(tag_dep, answer, current_user)
          end
        end
        html += '</td></tr>'
      end
      #### end code to show tag prompts ####
    end
    html += '</tr></table>'
    safe_join(["".html_safe, "".html_safe], html.html_safe)
  end

Test Plan

We are running Rspec tests to make sure the coverage is the same and manually checking the pages to make sure there are not any issues and the code is not breaking

RSpec Testing

The origin test about method "complete" didn't include the situation when the parameter "answer" isn't nil and the parameter "dropdown_or_scale" is "dropdown" or "scale". We added the test examples from 4 to 16 including the multiple for method "complete", "dropdown_criterion_question", "scale_criterion_question".

  • Test for method "complete" with or without answer and dorpdown_or_scale

Origin test

 describe "#complete" do
    it "returns the html " do
      html = criterion.complete(0, nil, 0, 5).to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div>")
    end
  end

New test

describe "#complete" do
    it "returns the html without answer and no dropdown or scale" do
      html = criterion.complete(0, nil, 0, 5).to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div>")
    end

    it "returns the html without answer and dropdown" do
      html = criterion.complete(0, nil, 0, 5, dropdown_or_scale = 'dropdown').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" ><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end

    it "returns the html with no comments answer and answer.answer outside questionnaire min and max and dropdown" do
      html = criterion.complete(0, answer_no_comments, 0, 5, dropdown_or_scale = 'dropdown').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =8><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end

    it "returns the html with comments answer and answer.answer between questionnaire min and max and dropdown" do
      html = criterion.complete(0, answer_comments, 0, 5, dropdown_or_scale = 'dropdown').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =3><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3 selected=\"selected\">3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea></td>")
    end

    it "returns the html without answer and scale" do
      html = criterion.complete(0, nil, 0, 5, dropdown_or_scale = 'scale').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end

    it "returns the html with no comments answer and answer.answer outside questionnaire min and max and scale" do
      html = criterion.complete(0, answer_no_comments, 0, 5, dropdown_or_scale = 'scale').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"8\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end

    it "returns the html with comments answer and answer.answer between questionnaire min and max and scale" do
      html = criterion.complete(0, answer_comments, 0, 5, dropdown_or_scale = 'scale').to_s
      expect(html).to eq("<div><label for=\"responses_0\">test txt</label></div><input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"3\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea>")
    end

  end
  • Test for method "dropdown_criterion_question" with or without answer
describe "#dropdown_criterion_question" do
    it "returns the html without answer" do
      html = criterion.dropdown_criterion_question(0, nil, 0, 5).to_s
      expect(html).to eq("<div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" ><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end
    
    it "returns the html with no comments answer and answer.answer outside questionnaire min and max" do
      html = criterion.dropdown_criterion_question(0, answer_no_comments, 0, 5).to_s
      expect(html).to eq("<div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =8><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3>3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea></td>")
    end

    it "returns the html with comments in answer and answer.answer between questionnaire min and max" do
      html = criterion.dropdown_criterion_question(0, answer_comments, 0, 5).to_s
      expect(html).to eq("<div><select id=\"responses_0_score\" name=\"responses[0][score]\" class=\"review-rating\" data-current-rating =3><option value = ''>--</option><option value=0>0</option><option value=1>1</option><option value=2>2</option><option value=3 selected=\"selected\">3</option><option value=4>4</option><option value=5>5</option></select></div><br><br><textarea id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea></td>")
    end
  end
  • Test for method "scale_criterion_question" with or without answer
describe "#scale_criterion_question" do
    it "returns the html without answer" do
      html = criterion.scale_criterion_question(0, nil, 0, 5).to_s
      expect(html).to eq("<input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end

    it "returns the html with no comments answer and answer.answer outside questionnaire min and max" do
      html = criterion.scale_criterion_question(0, answer_no_comments, 0, 5).to_s
      expect(html).to eq("<input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"8\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\"></textarea>")
    end

    it "returns the html with comments answer and answer.answer between questionnaire min and max" do
      html = criterion.scale_criterion_question(0, answer_comments, 0, 5).to_s
      expect(html).to eq("<input id=\"responses_0_score\" name=\"responses[0][score]\" type=\"hidden\"value=\"3\"><table><tr><td width=\"10%\"></td><td width=\"10%\"><label>0</label></td><td width=\"10%\"><label>1</label></td><td width=\"10%\"><label>2</label></td><td width=\"10%\"><label>3</label></td><td width=\"10%\"><label>4</label></td><td width=\"10%\"><label>5</label></td><td width=\"10%\"></td></tr><tr><td width=\"10%\"></td><td width=\"10%\"><input type=\"radio\" id=\"0\" value=\"0\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"1\" value=\"1\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"2\" value=\"2\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"3\" value=\"3\" name=\"Radio_1\"checked=\"checked\"></td><td width=\"10%\"><input type=\"radio\" id=\"4\" value=\"4\" name=\"Radio_1\"></td><td width=\"10%\"><input type=\"radio\" id=\"5\" value=\"5\" name=\"Radio_1\"></td><script>jQuery(\"input[name=Radio_1]:radio\").change(function() {var response_score = jQuery(\"#responses_0_score\");var checked_value = jQuery(\"input[name=Radio_1]:checked\").val();response_score.val(checked_value);});</script><td width=\"10%\"></td><td width=\"10%\"></td></tr></table><textarea cols=70 rows=1 id=\"responses_0_comments\" name=\"responses[0][comment]\" class=\"tinymce\">text comments</textarea>")
    end
  end
  • All existing tests passed

Manual Testing

Test Log in Website url: http://152.46.19.120:8080/
Log in: instructor6
Password: password
Log in: student2064
Password: password

Test criterion works well in questionnaire

1. After logging in as a student, click the "Assignments"
2. Choose an assignment, like "Backchannel application review"
3. You can choose "Your scores" to see the review results
4. Click Criterion 1, you can see all the reviews for this criterion.
5. Click "toggle question list", you can see all criterion questions.
6. After logging in as an instructor, choose "Questionnaires" tag
7. Click on the name Review: This should show a drop-down showing different reviews made.
8. Choose a review, select the edit icon to the right.
8. Change dropbox of question type to Criterion, select Add "2" more. Press the add button.
9. Edit question content: "Test question Textarea", "Test question Criterion1", and "Test question Criterion2" in that order.
10. You can also change the question content for exited criterion question

These manual tests show criterion works well. Feel free to try your own test cases.