CSC/ECE 517 Fall 2016/E1673. Refactor question type.rb

Background

Expertiza is an open source software to create reusable learning objects through peer review. It also supports team projects, and the submission of almost any document type, including URLs and wiki pages. It has following features:

• Enables students working together in a class and a topic
• Enables studnets to do peer review and improve both him/her and others' learn experience

Reason for Refractor

The purpose of this work is to rewrite part of the codes in the Criterion.rb, Scale.rb, Checkbox.rb, Dropdown.rb, Text_response.rb, Text_area.rb, Text_field.rb. This is because some of codes have bad writing sytle, or not followingg OO language rules, which confuse others to understand these code. The detail of the requirements are list below:

• Line is too long
• Some code found in other location and should put them in the parent class
• Method has too many lines
• Some of the code will cause security risk
• Using the old style validations
• Using a lower efficent way to do the loop

All of these requirements are based on a analysis software called code climate

Code Changes

text_field.rb

Although it increase the number of classes, the length of function decrease and it's easy to read than before

1. Short the long line

2. Improve security by using safe_join

Previous

html.html_safe

Changed

safe_join([" ".html_safe, " ".html_safe], html.html_safe)

3. Short long method

Previous

html += '<label for="responses_' + count.to_s + '">' + self.txt + '</label>'
   html += '<input id="responses_' + count.to_s + '_score" name="responses[' + count.to_s + '][score]" type="hidden" value="">'
   html += '<input id="responses_' + count.to_s + '_comments" label=' + self.txt + ' name="responses[' + count.to_s + '][comment]" size=' + self.size.to_s + ' type="text"'
   html += 'value="' + answer.comments unless answer.nil?
   html += '">'

html += '

' if self.type == 'TextField' and self.break_before == false def complete(count, answer = nil) html = if self.type == 'TextField' and self.break_before == true html += combine(count, answer) end Changed def combline(count, answer = nil) html = complete_helper_response_scores(count) + complete_helper_response_comments(count) html += complete_helper_text(answer) + complete_helper_response_comments html += complete_helper_final_judge html end

text_response.rb

It makes codes more readable. We also choose safe_join instead of html_safe to prevent security risks. However, we must use html_safe to keep the character encoding.

1. Use new style validations Previous

validates_presence_of :size

Changed

validates :size, presence: true

2. Short the long line

3. Improve security by safe_join

Preivious

html.html_safe

Changed

safe_join(["".html_safe, "".html_safe], html.html_safe) 4. Short long method Preivious def edit(_count) html = '' html += '<a rel="nofollow" data-method="delete" href="/questions/' + self.id.to_s + '">Remove</a>' html += '<input size="6" value="' + self.seq.to_s + '" name="question[' + self.id.to_s + '][seq]" id="question_' + self.id.to_s + '_seq" type="text">' html += '<textarea cols="50" rows="1" name="question[' + self.id.to_s + '][txt]" id="question_' + self.id.to_s + '_txt" placeholder="Edit question content here">' + self.txt + '</textarea>' html += '<input size="10" disabled="disabled" value="' + self.type + '" name="question[' + self.id.to_s + '][type]" id="question_' + self.id.to_s + '_type" type="text">' html += '' html += 'text area size <input size="6" value="' + self.size.to_s + '" name="question[' + self.id.to_s + '][size]" id="question_' + self.id.to_s + '_size" type="text">' html += '' html.html_safe end Changed def edit(_count) html = edit_link + edit_question_lable html += edit_question_textarea + edit_question_textarea_size safe_join(["".html_safe, "".html_safe], html.html_safe) end

scale.rb

1. Use each loop instead of for loop

2. Short the long line

3. Use safe_join to be more security

4. Delete the code similar with other file and put the duplicate into parent class.

5. Modify some method that is too long

checkbox.rb

1. Short the long line

2. Modify some method that is too long

3. Use safe_join to be more security

criterion.rb

1. Improve security by using safe_join

2. Modify some method that is too long

3. Use new style validations

4. Use each loop instead of for loop

dropdown.rb

1. Use new style validations

2. Modify some method that is too long

3. Improve security by using safe_join

text_area.rb

1. Replace condition statements in one line

Previous

 if self.size.nil?
   cols = '70'
   ows = '1'
 elsif
   cols = self.size.split(',')[0]
   rows = self.size.split(',')[1]
 end

Changed

 def complete_get_cols_rows
   return '70', '1' if self.size.nil?
   [self.size.split(',')[0], self.size.split(',')[1]]
 end

2. Improve security by using safe_join

Previous

 html.html_safe

Changed

safe_join(["

".html_safe, "

".html_safe], html.html_safe) safe_join([" ".html_safe, " ".html_safe], html.html_safe)