Tidy Comment

Remove self.
Use each rather than for
Use {} block rather than do/end
Use self. rather than ClassName. for class method
Indent private as much as method definitions
Remove explicit return
Line Length
Use inline if
Move callback before method definitions
Use unless instead of if !

1 files changed, 16 insertions, 17 deletions

diff --git a/app/models/comment.rb b/app/models/comment.rb
index a62c086d5..4e5e10898 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -30,13 +30,14 @@ class Comment < ActiveRecord::Base
     validates_inclusion_of :comment_type, :in => [ 'request' ]
     validate :body_of_comment
 
+    after_save :event_xapian_update
+
     def body
         ret = read_attribute(:body)
-        if ret.nil?
-            return ret
-        end
+        return ret if ret.nil?
         ret = ret.strip
-        ret = ret.gsub(/(?:\n\s*){2,}/, "\n\n") # remove excess linebreaks that unnecessarily space it out
+        # remove excess linebreaks that unnecessarily space it out
+        ret = ret.gsub(/(?:\n\s*){2,}/, "\n\n")
         ret
     end
 
@@ -45,47 +46,45 @@ class Comment < ActiveRecord::Base
     end
 
     # So when takes changes it updates, or when made invisble it vanishes
-    after_save :event_xapian_update
     def event_xapian_update
-        for event in self.info_request_events
-            event.xapian_mark_needs_index
-        end
+        info_request_events.each { |event| event.xapian_mark_needs_index }
     end
 
     # Return body for display as HTML
     def get_body_for_html_display
-        text = self.body.strip
+        text = body.strip
         text = CGI.escapeHTML(text)
         text = MySociety::Format.make_clickable(text, :contract => 1)
         text = text.gsub(/\n/, '<br>')
-        return text.html_safe
+        text.html_safe
     end
 
     # When posting a new comment, use this to check user hasn't double submitted.
-    def Comment.find_existing(info_request_id, body)
+    def self.find_existing(info_request_id, body)
         # TODO: can add other databases here which have regexp_replace
         if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
             # Exclude spaces from the body comparison using regexp_replace
-            return Comment.find(:first, :conditions => [ "info_request_id = ? and regexp_replace(body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')", info_request_id, body ])
+            Comment.find(:first, :conditions => [ "info_request_id = ? and regexp_replace(body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')", info_request_id, body ])
         else
             # For other databases (e.g. SQLite) not the end of the world being space-sensitive for this check
-            return Comment.find(:first, :conditions => [ "info_request_id = ? and body = ?", info_request_id, body ])
+            Comment.find(:first, :conditions => [ "info_request_id = ? and body = ?", info_request_id, body ])
         end
     end
 
     def for_admin_column
         self.class.content_columns.each do |column|
-            yield(column.human_name, self.send(column.name), column.type.to_s, column.name)
+            yield(column.human_name, send(column.name), column.type.to_s, column.name)
         end
     end
 
-  private
+    private
 
     def body_of_comment
-        if self.body.empty? || self.body =~ /^\s+$/
+        if body.empty? || body =~ /^\s+$/
             errors.add(:body, _("Please enter your annotation"))
         end
-        if !MySociety::Validate.uses_mixed_capitals(self.body)
+
+        unless MySociety::Validate.uses_mixed_capitals(body)
             errors.add(:body, _('Please write your annotation using a mixture of capital and lower case letters. This makes it easier for others to read.'))
         end
     end