From beb41b77db4691b2ca230b708ba9f8c73fec74a1 Mon Sep 17 00:00:00 2001
From: Vesa-Pekka Palmu <vpalmu@depili.fi>
Date: Sat, 20 May 2017 16:48:34 +0300
Subject: [PATCH] Additional cops, mostly guard clauses

---
 .rubocop.yml                               |  9 ++++-
 app/controllers/slides_controller.rb       | 36 ++++++++---------
 app/controllers/tickets_controller.rb      |  5 +--
 app/helpers/displays_helper.rb             | 33 ++++++---------
 app/helpers/events_helper.rb               |  5 +--
 app/helpers/presentations_helper.rb        | 17 ++++----
 app/helpers/tickets_helper.rb              | 47 +++++++++-------------
 app/models/concerns/has_slidedata.rb       |  7 ++--
 app/models/concerns/has_svg_data.rb        | 18 ++++-----
 app/models/concerns/model_authorization.rb | 14 ++-----
 app/models/concerns/websocket_messages.rb  |  7 ++--
 app/models/display.rb                      | 14 +++----
 app/models/display_state.rb                | 27 ++++++-------
 app/models/event.rb                        |  7 +---
 app/models/master_groups/prize_group.rb    | 14 +++----
 app/models/schedule.rb                     |  7 ++--
 app/models/slide_template.rb               |  7 ++--
 app/models/slides/image_slide.rb           | 38 +++++++++--------
 app/models/slides/simple_slide.rb          | 16 ++++----
 app/models/slides/svg_slide.rb             |  7 +---
 app/models/user.rb                         | 24 ++++-------
 isk-server.rb                              |  5 +--
 script/rrd_monitoring.rb                   |  7 +---
 test/test_helper.rb                        |  8 ++--
 24 files changed, 160 insertions(+), 219 deletions(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 6242ef72..a75fd85a 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -144,4 +144,11 @@ Style/DotPosition:
   
 Style/EmptyElse:
   Enabled: true
-  EnforcedStyle: both
\ No newline at end of file
+  EnforcedStyle: both
+  
+Style/GuardClause:
+  Enabled: true
+  MinBodyLength: 2
+  
+Performance:
+  Enabled: true
\ No newline at end of file
diff --git a/app/controllers/slides_controller.rb b/app/controllers/slides_controller.rb
index d0a12b26..96e2b41c 100644
--- a/app/controllers/slides_controller.rb
+++ b/app/controllers/slides_controller.rb
@@ -363,6 +363,7 @@ def full
 
   # Send a given sized slide image
   def send_slide_image(size)
+    return unless stale?(last_modified: @slide.images_updated_at.utc, etag: @slide)
     case size
     when :full
       filename = @slide.full_filename
@@ -371,22 +372,19 @@ def send_slide_image(size)
     else
       filename = @slide.preview_filename
     end
-
-    if stale?(last_modified: @slide.images_updated_at.utc, etag: @slide)
-      respond_to do |format|
-        format.html do
-          # Set content headers to allow CORS
-          response.headers["Access-Control-Allow-Origin"] = "*"
-          response.headers["Access-Control-Request-Method"] = "GET"
-          if @slide.ready
-            send_file filename, disposition: "inline"
-          else
-            send_file(Rails.root.join("data", "no_image.jpg"),
-                      disposition: "inline")
-          end
+    respond_to do |format|
+      format.html do
+        # Set content headers to allow CORS
+        response.headers["Access-Control-Allow-Origin"] = "*"
+        response.headers["Access-Control-Request-Method"] = "GET"
+        if @slide.ready
+          send_file filename, disposition: "inline"
+        else
+          send_file(Rails.root.join("data", "no_image.jpg"),
+                    disposition: "inline")
         end
-        format.js { render :show }
       end
+      format.js { render :show }
     end
   end
 
@@ -400,14 +398,12 @@ def slide_params
   end
 
   def require_create
-    unless Slide.can_create? current_user
-      fail ApplicationController::PermissionDenied
-    end
+    return if Slide.can_create? current_user
+    fail ApplicationController::PermissionDenied
   end
 
   def require_admin
-    unless Slide.admin? current_user
-      fail ApplicationController::PermissionDenied
-    end
+    return if Slide.admin? current_user
+    fail ApplicationController::PermissionDenied
   end
 end
diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb
index ac8a38e0..86b3c101 100644
--- a/app/controllers/tickets_controller.rb
+++ b/app/controllers/tickets_controller.rb
@@ -98,8 +98,7 @@ def ticket_create_params
   end
 
   def require_admin
-    unless Ticket.admin? current_user
-      fail ApplicationController::PermissionDenied
-    end
+    return unless Ticket.admin? current_user
+    fail ApplicationController::PermissionDenied
   end
 end
diff --git a/app/helpers/displays_helper.rb b/app/helpers/displays_helper.rb
index 6a50e02d..164297b6 100644
--- a/app/helpers/displays_helper.rb
+++ b/app/helpers/displays_helper.rb
@@ -13,15 +13,14 @@ def late_display_warning(d)
 
   # Link to displays#destroy if user has sufficient access
   def display_destroy_button(d)
-    if d.admin?(current_user)
-      link_to icon("times-circle", "Delete"), display_path(d),
-              data: {
-                      confirm: "Are you sure you want to delete the display"\
-                               " \"#{d.name}\", this cannot be undone?"
-                    },
-              title: "Delete this display premanently",
-              method: :delete, class: "button warning"
-    end
+    return unless d.admin?(current_user)
+    link_to icon("times-circle", "Delete"), display_path(d),
+            data: {
+                    confirm: "Are you sure you want to delete the display"\
+                             " \"#{d.name}\", this cannot be undone?"
+                  },
+            title: "Delete this display premanently",
+            method: :delete, class: "button warning"
   end
 
   # Set the panel class based on display status
@@ -38,11 +37,8 @@ def display_class(display)
     else
       panel = "panel-danger"
     end
-    if display.live?
-      return "#{panel} display-live"
-    else
-      return panel
-    end
+    return "#{panel} display-live" if display.live?
+    return panel
   end
 
   # Render the display ping element
@@ -88,11 +84,8 @@ def display_current_slide(d)
 
   # Render the last_contact_at timestamp and the diff to current time
   def display_last_contact(d)
-    if d.last_contact_at
-      delta = Time.diff(Time.now, d.last_contact_at, "%h:%m:%s")[:diff]
-      return "#{l d.last_contact_at, format: :short} (#{delta} ago)"
-    else
-      return "UNKNOWN"
-    end
+    return "UNKNOWN" unless d.last_contact_at
+    delta = Time.diff(Time.now, d.last_contact_at, "%h:%m:%s")[:diff]
+    return "#{l d.last_contact_at, format: :short} (#{delta} ago)"
   end
 end
diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb
index 4a50d31f..a0ceafd7 100644
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@@ -33,9 +33,8 @@ def event_new_button
 
   # Check if this event is current one and if it is then set the class to 'success'
   def event_current_class(event)
-    if event.current
-      "success"
-    end
+    return unless event.current
+    "success"
   end
 
   # Select box for choosing the slide resolution for the event.
diff --git a/app/helpers/presentations_helper.rb b/app/helpers/presentations_helper.rb
index 9d874cb8..914b54c4 100644
--- a/app/helpers/presentations_helper.rb
+++ b/app/helpers/presentations_helper.rb
@@ -13,15 +13,14 @@ def duration_to_text(dur)
 
   # Link to displays#destroy if user has sufficient access
   def presentation_destroy_button(p)
-    if p.can_edit?(current_user)
-      link_to delete_link_text, presentation_path(p),
-              data: {
-                      confirm: "Are you sure you want to delete the presentation"\
-                               " \"#{p.name}\", this cannot be undone?"
-                    },
-              title: "Delete this presentation premanently",
-              method: :delete, class: "btn btn-danger"
-    end
+    return unless p.can_edit?(current_user)
+    link_to delete_link_text, presentation_path(p),
+            data: {
+                    confirm: "Are you sure you want to delete the presentation"\
+                             " \"#{p.name}\", this cannot be undone?"
+                  },
+            title: "Delete this presentation premanently",
+            method: :delete, class: "btn btn-danger"
   end
 
   # Button to edit a presentation
diff --git a/app/helpers/tickets_helper.rb b/app/helpers/tickets_helper.rb
index d94d2161..2c285561 100644
--- a/app/helpers/tickets_helper.rb
+++ b/app/helpers/tickets_helper.rb
@@ -7,11 +7,9 @@
 module TicketsHelper
   # Link to the object associated to this ticket
   def ticket_concerning(ticket)
-    if ticket.about.present?
-      return "#{ticket.about_type.capitalize}: #{ticket_object_link(ticket)}".html_safe
-    else
-      return "None"
-    end
+    return "None" unless ticket.about.present?
+    return "#{ticket.about_type.capitalize}:"\
+           " #{ticket_object_link(ticket)}".html_safe
   end
 
   # Render a link to the associated object on a ticket
@@ -80,14 +78,13 @@ def ticket_edit_button(ticket)
   end
 
   def ticket_close_link(ticket, html_class = nil)
-    if ticket.can_close? current_user
-      link_to icon("check-square-o", "Close"),
-              ticket_path(ticket,
-                          ticket: {
-                            status: Ticket::StatusClosed }),
-              method: :put,
-              class: html_class
-    end
+    return unless ticket.can_close? current_user
+    link_to icon("check-square-o", "Close"),
+            ticket_path(ticket,
+                        ticket: {
+                          status: Ticket::StatusClosed }),
+            method: :put,
+            class: html_class
   end
 
   def ticket_close_button(ticket)
@@ -95,13 +92,12 @@ def ticket_close_button(ticket)
   end
 
   def ticket_destroy_link(ticket, html_class = nil)
-    if ticket.admin? current_user
-      link_to icon("times-circle", "Delete"),
-              ticket_path(ticket),
-              class: html_class,
-              method: :delete,
-              data: { confirm: "Are you sure you want to permanently delete this ticket?" }
-    end
+    return unless ticket.admin? current_user
+    link_to icon("times-circle", "Delete"),
+            ticket_path(ticket),
+            class: html_class,
+            method: :delete,
+            data: { confirm: "Are you sure you want to permanently delete this ticket?" }
   end
 
   def ticket_destroy_button(ticket)
@@ -129,12 +125,9 @@ def ticket_kind(ticket)
 private
 
   def ticket_open_count
-    if Ticket.current.open.count > 0
-      html = icon "ticket"
-      html << Ticket.ticket.open.count.to_s
-      return html
-    else
-      return ""
-    end
+    return "" unless Ticket.current.open.count > 0
+    html = icon "ticket"
+    html << Ticket.ticket.open.count.to_s
+    return html
   end
 end
diff --git a/app/models/concerns/has_slidedata.rb b/app/models/concerns/has_slidedata.rb
index 305bcdfa..699d4e24 100644
--- a/app/models/concerns/has_slidedata.rb
+++ b/app/models/concerns/has_slidedata.rb
@@ -78,10 +78,9 @@ def default_slidedata
   end
 
   def write_slidedata
-    unless self.new_record?
-      File.open(self.data_filename,  "w") do |f|
-        f.write sanitalize_slidedata(@_slidedata).to_yaml
-      end
+    return if self.new_record?
+    File.open(self.data_filename,  "w") do |f|
+      f.write sanitalize_slidedata(@_slidedata).to_yaml
     end
   end
 end
diff --git a/app/models/concerns/has_svg_data.rb b/app/models/concerns/has_svg_data.rb
index 3c698992..9110e582 100644
--- a/app/models/concerns/has_svg_data.rb
+++ b/app/models/concerns/has_svg_data.rb
@@ -33,13 +33,10 @@ def svg_data
   # We also mark the slide as not ready because the picture isn't current anymore and needs
   # to be regenerated.
   def svg_data=(svg)
-    if self.svg_data != svg
-
-      @_svg_data = svg
-      write_svg_data
-
-      self.ready = false
-    end
+    return unless self.svg_data != svg
+    @_svg_data = svg
+    write_svg_data
+    self.ready = false
   end
 
   # Filename to store the svg in.
@@ -53,10 +50,9 @@ def svg_filename
   # We need to check if record is new, because we don't know the filename for the svg
   # before the record is saved.
   def write_svg_data
-    unless self.new_record?
-      File.open(self.svg_filename, "wb") do |f|
-        f.write @_svg_data
-      end
+    return if self.new_record?
+    File.open(self.svg_filename, "wb") do |f|
+      f.write @_svg_data
     end
   end
 
diff --git a/app/models/concerns/model_authorization.rb b/app/models/concerns/model_authorization.rb
index b42ff60c..e0756303 100644
--- a/app/models/concerns/model_authorization.rb
+++ b/app/models/concerns/model_authorization.rb
@@ -61,20 +61,14 @@ def can_create?(user)
 
     # List of all displays a user can add overrides on
     def can_override(user)
-      if user.has_role?([self.auth_roles[:admin], self.auth_roles[:override]])
-        return relation
-      else
-        return self.joins(:authorized_users).where(users: { id: user.id })
-      end
+      return relation if user.has_role?([self.auth_roles[:admin], self.auth_roles[:override]])
+      return self.joins(:authorized_users).where(users: { id: user.id })
     end
 
     # List of all objects user can edit
     def can_edit(user)
-      if user.has_role?(self.auth_roles[:admin])
-        return relation
-      else
-        self.joins(:authorized_users).where("users.id = ?", user.id)
-      end
+      return relation if user.has_role?(self.auth_roles[:admin])
+      self.joins(:authorized_users).where("users.id = ?", user.id)
     end
 
     # Does user have admin priviledges on all objects of this type?
diff --git a/app/models/concerns/websocket_messages.rb b/app/models/concerns/websocket_messages.rb
index a8c95688..2dc214c1 100644
--- a/app/models/concerns/websocket_messages.rb
+++ b/app/models/concerns/websocket_messages.rb
@@ -60,10 +60,9 @@ def send_messages(event)
       display_datas
     end
 
-    if self.previous_changes.include?("images_updated_at") && event == :update
-      Rails.logger.debug "-> Slide image has been updated, sending notifications"
-      self.updated_image_notifications
-    end
+    return unless self.previous_changes.include?("images_updated_at") && event == :update
+    Rails.logger.debug "-> Slide image has been updated, sending notifications"
+    self.updated_image_notifications
   end
 
   def get_channel
diff --git a/app/models/display.rb b/app/models/display.rb
index a12613d0..ad12c2c7 100644
--- a/app/models/display.rb
+++ b/app/models/display.rb
@@ -143,11 +143,8 @@ def self.late
 
   # Is this display more than Timeout minutes late?
   def late?
-    if self.last_contact_at
-      return Time.diff(Time.now, self.last_contact_at, "%m")[:diff].to_i > Timeout
-    else
-      return false
-    end
+    return false unless self.last_contact_at
+    return Time.diff(Time.now, self.last_contact_at, "%m")[:diff].to_i > Timeout
   end
 
   # Is the display live, ie. visible to the general audience
@@ -218,10 +215,9 @@ def ping
 
   # Create the associated display state as needed
   def create_state
-    if self.display_state.nil?
-      ds = DisplayState.new
-      self.display_state = ds
-    end
+    return unless self.display_state.nil?
+    ds = DisplayState.new
+    self.display_state = ds
   end
 
   # If display is in manual control also stop accepting overrides
diff --git a/app/models/display_state.rb b/app/models/display_state.rb
index eadda3b6..ba80ddda 100644
--- a/app/models/display_state.rb
+++ b/app/models/display_state.rb
@@ -33,20 +33,19 @@ def displays
 
   # Send error notifications if we are in error state
   def send_error_notifications
-    if self.status == "error"
-      if self.display.error_tickets.open.present?
-        msg = self.display.error_tickets.open.last!.description.lines.last
-      else
-        msg = "Error has occured!"
-      end
-      data = {
-        id: self.display_id,
-        message: msg
-      }
-      Rails.logger.error "Error has occured on display #{self.display_id} with message: '#{msg}'"
-      msg = IskMessage.new("display", "error", data)
-      msg.send
-      msg.send(self.display.websocket_channel)
+    return unless self.status == "error"
+    if self.display.error_tickets.open.present?
+      msg = self.display.error_tickets.open.last!.description.lines.last
+    else
+      msg = "Error has occured!"
     end
+    data = {
+      id: self.display_id,
+      message: msg
+    }
+    Rails.logger.error "Error has occured on display #{self.display_id} with message: '#{msg}'"
+    msg = IskMessage.new("display", "error", data)
+    msg.send
+    msg.send(self.display.websocket_channel)
   end
 end
diff --git a/app/models/event.rb b/app/models/event.rb
index c9903f0a..bb3ca892 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -141,11 +141,8 @@ def simple_editor_settings
 
   # Set the size for full slide pictures. Checks that the resolution is supported.
   def picture_size=(size)
-    if SupportedResolutions.include? size
-      self[:resolution] = SupprotedResolutions.index(size)
-    else
-      raise ArgumentError, "Resolution not supported"
-    end
+    raise ArgumentError, "Resolution not supported" unless SupportedResolutions.include? size
+    self[:resolution] = SupprotedResolutions.index(size)
   end
 
   # Returns a hash containing the set picture sizes.
diff --git a/app/models/master_groups/prize_group.rb b/app/models/master_groups/prize_group.rb
index 70e451c1..3e8073a1 100644
--- a/app/models/master_groups/prize_group.rb
+++ b/app/models/master_groups/prize_group.rb
@@ -152,18 +152,14 @@ def empty_template_data
   end
 
   def data_filename
-    if self.id
-      return Rails.root.join("data", "prizes", "prize_group_#{id}")
-    else
-      return nil
-    end
+    return nill unless self.id
+    return Rails.root.join("data", "prizes", "prize_group_#{id}")
   end
 
   def write_data
-    unless self.new_record?
-      File.open(data_filename,  "w") do |f|
-        f.write self.data.to_yaml
-      end
+    return if self.new_record?
+    File.open(data_filename,  "w") do |f|
+      f.write self.data.to_yaml
     end
   end
 end
diff --git a/app/models/schedule.rb b/app/models/schedule.rb
index 61af37e5..8c2b51c7 100644
--- a/app/models/schedule.rb
+++ b/app/models/schedule.rb
@@ -151,11 +151,10 @@ def generate_next_up_slide
   def find_or_initialize_next_up_slide
     if self.next_up_group.slides.where(type: ScheduleSlide.sti_name).first.present?
       return self.next_up_group.slides.where(type: ScheduleSlide.sti_name).first!
-    else
-      slide = ScheduleSlide.new
-      self.next_up_group.slides << slide
-      return slide
     end
+    slide = ScheduleSlide.new
+    self.next_up_group.slides << slide
+    return slide
   end
 
   # Convenience method for getting the count of schedule slides in our slidegroup
diff --git a/app/models/slide_template.rb b/app/models/slide_template.rb
index 42e3893f..0d408bae 100644
--- a/app/models/slide_template.rb
+++ b/app/models/slide_template.rb
@@ -116,10 +116,9 @@ def generate_settings(svg)
   # we use binary mode here to prevent ascii conversions..
   # FIXME: set viewBox on import, so web preview scales properly!
   def write_template
-    unless self.new_record?
-      File.open(self.filename, "wb") do |f|
-        f.write @_template
-      end
+    return if self.new_record?
+    File.open(self.filename, "wb") do |f|
+      f.write @_template
     end
   end
 
diff --git a/app/models/slides/image_slide.rb b/app/models/slides/image_slide.rb
index b3aea390..b8d7593e 100644
--- a/app/models/slides/image_slide.rb
+++ b/app/models/slides/image_slide.rb
@@ -37,28 +37,27 @@ def image=(image)
 
   # A after save hook to move the possible new image into its place
   def save_image
-    if @_image_file
-      FileUtils.move @_image_file.path, self.original_filename
-      @_image_file = nil
-    end
+    return unless @_image_file
+    FileUtils.move @_image_file.path, self.original_filename
+    @_image_file = nil
   end
 
   # Validate the uploaded image.
   def check_image
-    if @_image_file
-      # Verify image integrity
-      command = "identify #{@_image_file.path} &> /dev/null"
-      unless system command
-        @_image_file.unlink
-        @_image_file = nil
-        errors.add :image, "wasn't a valid image file."
-      end
-    end
+    return unless @_image_file
+    # Verify image integrity
+    command = "identify #{@_image_file.path} &> /dev/null"
+    return if system command
+
+    # Verify error
+    @_image_file.unlink
+    @_image_file = nil
+    errors.add :image, "wasn't a valid image file."
   end
 
   # Validates that the background color is ok
   def check_bg_color
-    unless slidedata[:background].match /\A#(?:[0-9a-f]{3})(?:[0-9a-f]{3})?\z/
+    unless slidedata[:background].match(/\A#(?:[0-9a-f]{3})(?:[0-9a-f]{3})?\z/)
       errors.add :backgroud, "must be valid css hex color"
     end
   end
@@ -94,11 +93,10 @@ def generate_full_image
     tmp_file = Tempfile.new("isk-image")
     command = "convert #{self.original_filename} -resize #{geo_str}"
     command << " -background #{bg_color.shellescape} -gravity center -extent #{size} png:#{tmp_file.path}"
-    if system command
-      return compare_new_image(tmp_file)
-    else
-      tmp_file.unlink
-      raise Slide::ImageError, "Error generating full size slide image!"
-    end
+    return compare_new_image(tmp_file) if system command
+
+    # Image generation error
+    tmp_file.unlink
+    raise Slide::ImageError, "Error generating full size slide image!"
   end
 end
diff --git a/app/models/slides/simple_slide.rb b/app/models/slides/simple_slide.rb
index 45c6a9e6..0513736a 100644
--- a/app/models/slides/simple_slide.rb
+++ b/app/models/slides/simple_slide.rb
@@ -257,15 +257,13 @@ def self.set_text(element, text, text_x, color = nil, size = nil, align = nil)
   end
 
   def self.row_x(align, margins)
-    if align
-      case align.strip.downcase
-      when "right"
-        return margins.last
-      when "centered"
-        return (margins.first + margins.last) / 2
-      else
-        return margins.first
-      end
+    return margins.first unless align
+
+    case align.strip.downcase
+    when "right"
+      return margins.last
+    when "centered"
+      return (margins.first + margins.last) / 2
     else
       return margins.first
     end
diff --git a/app/models/slides/svg_slide.rb b/app/models/slides/svg_slide.rb
index 5e66a62a..d71d6785 100644
--- a/app/models/slides/svg_slide.rb
+++ b/app/models/slides/svg_slide.rb
@@ -16,11 +16,8 @@ class SvgSlide < Slide
   def generate_full_image
     tmp_file = Tempfile.new("isk-image")
     output = `#{inkscape_command_line(tmp_file)}`
-    if $?.to_i == 0
-      return compare_new_image(tmp_file)
-    else
-      raise Slide::ImageError, "Error converting the slide svg into PNG\nInkscape output:\n#{output}"
-    end
+    return compare_new_image(tmp_file) if $?.to_i == 0
+    raise Slide::ImageError, "Error converting the slide svg into PNG\nInkscape output:\n#{output}"
   end
 end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index d6967355..b0d4ad0d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -28,14 +28,13 @@ def admin?
 
   def has_role?(request)
     return true if self.admin?
-    if request.is_a? Array
-      request.each do |r|
-        return true if self.roles.where(role: r).count > 0
-      end
-      return false
-    else
+    unless request.is_a? Array
       return self.roles.where(role: request).count > 0
     end
+    request.each do |r|
+      return true if self.roles.where(role: r).count > 0
+    end
+    return false
   end
 
   def roles_text
@@ -63,20 +62,13 @@ def password
   end
 
   def authenticate(passwd)
-    if self[:password] == Digest::SHA1.hexdigest(passwd << self[:salt])
-      return true
-    else
-      return false
-    end
+    self[:password] == Digest::SHA1.hexdigest(passwd << self[:salt])
   end
 
   def self.authenticate(username, passwd)
     user = User.where(username: username).first
-    if user && user.authenticate(passwd)
-      return user
-    else
-      return nil
-    end
+    return user if user && user.authenticate(passwd)
+    return nil
   end
 
   def cache_tag
diff --git a/isk-server.rb b/isk-server.rb
index 2aed7ced..1919cb49 100755
--- a/isk-server.rb
+++ b/isk-server.rb
@@ -35,9 +35,8 @@ def check_deps
   if inkscape_version == "0.48.5"
     abort "ERROR: Inkscape version 0.48.5 (in debian Jessie) has non-sane text element postioning, use 0.91 from jessie-backports instead".red
   end
-  unless find_executable("convert") && find_executable("identify")
-    abort "ERROR: ISK needs ImageMagick cmd line utilities (convert and indentify)".red
-  end
+  return if find_executable("convert") && find_executable("identify")
+  abort "ERROR: ISK needs ImageMagick cmd line utilities (convert and indentify)".red
 end
 
 def start_service(process)
diff --git a/script/rrd_monitoring.rb b/script/rrd_monitoring.rb
index 642bddb3..ef56e87e 100755
--- a/script/rrd_monitoring.rb
+++ b/script/rrd_monitoring.rb
@@ -43,11 +43,8 @@ def extract_pid(f)
     pid = File.read(@pid_path.join(f).to_s).to_i
 
     # Check if the pid is running
-    if system "ps -p #{pid} 1>/dev/null"
-      return pid
-    else
-      return nil
-    end
+    return pid if system "ps -p #{pid} 1>/dev/null"
+    return nil
   end
 
   def create_rrd_for_process(rrd_file)
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 90a0eae8..8a825d8b 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -42,10 +42,10 @@ def init_slide_files(slide)
       FileUtils.chmod 0600, slide.svg_filename.to_s
     end
 
-    if slide.is_a? SimpleSlide
-      FileUtils.cp simple_slidedatafile, slide.data_filename.to_s
-      FileUtils.chmod 0600, slide.data_filename.to_s
-    end
+    return unless slide.is_a? SimpleSlide
+
+    FileUtils.cp simple_slidedatafile, slide.data_filename.to_s
+    FileUtils.chmod 0600, slide.data_filename.to_s
   end
 
   # During testing we will end up generating slide datafiles in a temporary location.