2

I have this method:

  def self.get_image(product_id)
    self.initialize

    product_id=product_id.to_s
    if @image_db.key?(product_id)
      if Time.now.to_i - @image_db[product_id][':cached_at'] > @refresh_period
        puts Time.now.to_i - @image_db[product_id][':cached_at']
        self.cache_image(product_id)
      else
        return @image_db[product_id][':uri']
      end
    else
      self.cache_image(product_id)
    end
  end

and I am getting rubocop errors to use a guard clause instead of an if-else statement. What would be the best way to do this?

I was contemplating this code:

  def self.get_image(product_id)
    self.initialize

    product_id=product_id.to_s
    return if @image_db.key?(product_id)
    return if Time.now.to_i - @image_db[product_id][':cached_at'] > @refresh_period
    puts Time.now.to_i - @image_db[product_id][':cached_at']
    self.cache_image(product_id)
  end

but this line would never be called:

return @image_db[product_id][':uri']
sawa
  • 165,429
  • 45
  • 277
  • 381
snowflakekiller
  • 3,428
  • 6
  • 27
  • 45

1 Answers1

5

and I am getting rubocop errors to use a guard clause instead of an if-else statement... what would be the best way to do this?

First of all read carefully few articles on what guard clause is.

Here is your method refactored to use guard clauses:

def self.get_image(product_id)
  initialize

  product_id = product_id.to_s

  return cache_image(product_id)       unless @image_db.key?(product_id)
  return @image_db[product_id][':uri'] unless Time.now.to_i - @image_db[product_id][':cached_at'] > @refresh_period
  puts Time.now.to_i - @image_db[product_id][':cached_at']
  cache_image(product_id)
end

I would probably move some parts of method out to simplify it a bit:

def self.get_image(product_id)
  initialize
  product_id = product_id.to_s
  return cache_image(product_id)       unless @image_db.key?(product_id)
  return @image_db[product_id][':uri'] unless cached_at_gttn_refresh_period?(product_id)
  puts Time.now.to_i - @image_db[product_id][':cached_at']
  cache_image(product_id)
end

private

def cached_at_gttn_refresh_period?(product_id)
  Time.now.to_i - @image_db[product_id][':cached_at'] > @refresh_period
end
Andrey Deineko
  • 51,333
  • 10
  • 112
  • 145
  • 1
    Thank you Andrey for this, was enlightening. For anybody else who needs help with guard clauses, along with Andrey's answer, I found these links that helped me out a lot: http://www.thechrisoshow.com/2009/02/16/using-guard-clauses-in-your-ruby-code/ http://stackoverflow.com/questions/31943125/how-to-correctly-use-guard-clause-in-ruby http://www.rubydoc.info/github/bbatsov/rubocop/Rubocop/Cop/Style/GuardClause – snowflakekiller Feb 16 '16 at 14:09